Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

rename the config and make it non-internal.

Why are the changes needed?

Now we fail the query if duplicated map keys are detected, and provide a legacy config to deduplicate it. However, we must provide a way to get users out of this situation, instead of just rejecting to run the query. This exit strategy should always be there, while legacy config indicates that it may be removed someday.

Does this PR introduce any user-facing change?

no, just rename a config which was added in 3.0

How was this patch tested?

add more tests for the fail behavior.

@cloud-fan
Copy link
Contributor Author

cc @xuanyuanking @gatorsmile

@SparkQA
Copy link

SparkQA commented Mar 3, 2020

Test build #119232 has finished for PR 27772 at commit 80c7450.

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

.booleanConf
.createWithDefault(false)

val LEGACY_ALLOW_DUPLICATED_MAP_KEY =
Copy link
Member

@viirya viirya Mar 3, 2020

Choose a reason for hiding this comment

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

We need to update sql-migration-guide doc too. We already documented spark.sql.legacy.allowDuplicatedMapKeys there.

Copy link
Member

Choose a reason for hiding this comment

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

+1,
Also find another mistake I made in the migration guide, fixed in #27782, please take a look.

@SparkQA
Copy link

SparkQA commented Mar 4, 2020

Test build #119283 has finished for PR 27772 at commit b201c9e.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Mar 4, 2020

Test build #119291 has finished for PR 27772 at commit b201c9e.

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

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Mar 4, 2020

Test build #119312 has finished for PR 27772 at commit b201c9e.

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

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Mar 4, 2020

Test build #119320 has finished for PR 27772 at commit b201c9e.

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

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Mar 4, 2020

Test build #119318 has finished for PR 27772 at commit b201c9e.

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

@SparkQA
Copy link

SparkQA commented Mar 4, 2020

Test build #119327 has finished for PR 27772 at commit b201c9e.

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

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Mar 5, 2020

Merged to master, and branch-3.0.

HyukjinKwon pushed a commit that referenced this pull request Mar 5, 2020
rename the config and make it non-internal.

Now we fail the query if duplicated map keys are detected, and provide a legacy config to deduplicate it. However, we must provide a way to get users out of this situation, instead of just rejecting to run the query. This exit strategy should always be there, while legacy config indicates that it may be removed someday.

no, just rename a config which was added in 3.0

add more tests for the fail behavior.

Closes #27772 from cloud-fan/map.

Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
### What changes were proposed in this pull request?

rename the config and make it non-internal.

### Why are the changes needed?

Now we fail the query if duplicated map keys are detected, and provide a legacy config to deduplicate it. However, we must provide a way to get users out of this situation, instead of just rejecting to run the query. This exit strategy should always be there, while legacy config indicates that it may be removed someday.

### Does this PR introduce any user-facing change?

no, just rename a config which was added in 3.0

### How was this patch tested?

add more tests for the fail behavior.

Closes apache#27772 from cloud-fan/map.

Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants