Skip to content

[SPARK-25832][SQL][BRANCH-2.4] Revert newly added map related functions - #22827

Closed
gatorsmile wants to merge 7 commits into
apache:branch-2.4from
gatorsmile:revertMap2.4
Closed

[SPARK-25832][SQL][BRANCH-2.4] Revert newly added map related functions#22827
gatorsmile wants to merge 7 commits into
apache:branch-2.4from
gatorsmile:revertMap2.4

Conversation

@gatorsmile

@gatorsmilegatorsmile commented Oct 25, 2018•
edited
Loading

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

How was this patch tested?

The existing tests.

@gatorsmile

gatorsmile commented Oct 25, 2018

Copy link
Copy Markdown
MemberAuthor

@cloud-fan@dongjoon-hyun@rxin@felixcheung

@gatorsmile

gatorsmile commented Oct 25, 2018

Copy link
Copy Markdown
MemberAuthor

@felixcheung@mengxr The last commit f1f3d0b is for R related revert. Please take a look

@dongjoon-hyundongjoon-hyun mentioned this pull request Oct 25, 2018
Closed
dongjoon-hyun
dongjoon-hyun reviewed Oct 25, 2018
View reviewed changes
Comment threadsql/core/src/test/resources/sql-tests/inputs/higher-order-functions.sql Outdated
@@ -65,21 +65,3 @@ create or replace temporary view nested as values
(1, map(1, 1, 2, 2, 3, 3)),
(2, map(4, 4, 5, 5, 6, 6))
as t(x, ys);

@dongjoon-hyundongjoon-hyunOct 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should revert line 64~67 because they are introduced by SPARK-23939 Add transform_keys function.

@dongjoon-hyun

dongjoon-hyun commented Oct 25, 2018

Copy link
Copy Markdown
Member

Also, please remove public static final int WORD_SIZE = 8; in UnsafeRow.java. It's added by map_entries.

@gatorsmile

gatorsmile commented Oct 25, 2018

Copy link
Copy Markdown
MemberAuthor

@dongjoon-hyun I think that can be kept.

dongjoon-hyun
dongjoon-hyun reviewed Oct 25, 2018
View reviewed changes
Comment threadsql/core/src/test/resources/sql-tests/inputs/higher-order-functions.sql
create or replace temporary view nested as values
(1, map(1, 1, 2, 2, 3, 3)),
(2, map(4, 4, 5, 5, 6, 6))
as t(x, ys);

@dongjoon-hyundongjoon-hyunOct 25, 2018•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

higher-order-functions.sql.out together?

@gatorsmilegatorsmileOct 25, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That has been removed already

@dongjoon-hyundongjoon-hyunOct 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Right!

@dongjoon-hyun

dongjoon-hyun commented Oct 25, 2018

Copy link
Copy Markdown
Member

@gatorsmile . Could you put [BRANCH-2.4] into PR title?

@SparkQA

SparkQA commented Oct 25, 2018

Copy link
Copy Markdown

Test build #98029 has finished for PR 22827 at commit f1f3d0b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmilegatorsmile changed the title [SPARK-25832][SQL] Revert newly added map related functions[SPARK-25832][SQL][BRANCH-2.4] Revert newly added map related functionsOct 25, 2018
dongjoon-hyun
dongjoon-hyun approved these changes Oct 25, 2018
View reviewed changes

@dongjoon-hyundongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.
Thank you, @gatorsmile !

@SparkQA

SparkQA commented Oct 25, 2018

Copy link
Copy Markdown

Test build #98034 has finished for PR 22827 at commit 8bdf02e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan

cloud-fan commented Oct 25, 2018

Copy link
Copy Markdown
Contributor

thanks, merging to 2.4!

asfgit pushed a commit that referenced this pull request Oct 25, 2018
@gatorsmile@cloud-fan
[SPARK-25832][SQL][BRANCH-2.4] Revert newly added map related functions
a9f200e
## What changes were proposed in this pull request?
- Revert [SPARK-23935][SQL] Adding map_entries function: #21236
- Revert [SPARK-23937][SQL] Add map_filter SQL function: #21986
- Revert [SPARK-23940][SQL] Add transform_values SQL function: #22045
- Revert [SPARK-23939][SQL] Add transform_keys function: #22013
- Revert [SPARK-23938][SQL] Add map_zip_with function: #22017
- Revert the changes of map_entries in [SPARK-24331][SPARKR][SQL] Adding arrays_overlap, array_repeat, map_entries to SparkR: #21434
## How was this patch tested?
The existing tests.
Closes#22827 from gatorsmile/revertMap2.4.
Authored-by: gatorsmile <gatorsmile@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@HyukjinKwon

HyukjinKwon commented Oct 26, 2018

Copy link
Copy Markdown
Member

LGTM too

felixcheung
felixcheung approved these changes Oct 26, 2018
View reviewed changes

@felixcheungfelixcheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R LGTM

@MisterTea

MisterTea commented Jun 21, 2019

Copy link
Copy Markdown

@gatorsmile Hey, why were these functions reverted?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixcheungfelixcheungfelixcheung approved these changes
@dongjoon-hyundongjoon-hyundongjoon-hyun approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@gatorsmile@dongjoon-hyun@SparkQA@cloud-fan@HyukjinKwon@MisterTea@felixcheung