Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-23316][SQL] AnalysisException after max iteration reached for IN query #20548

Closed
wants to merge 1 commit into from

Conversation

bogdanrdc
Copy link
Contributor

What changes were proposed in this pull request?

Added flag ignoreNullability to DataType.equalsStructurally.
The previous semantic is for ignoreNullability=false.
When ignoreNullability=true equalsStructurally ignores nullability of contained types (map key types, value types, array element types, structure field types).
In.checkInputTypes calls equalsStructurally to check if the children types match. They should match regardless of nullability (which is just a hint), so it is now called with ignoreNullability=true.

How was this patch tested?

New test in SubquerySuite

@SparkQA
Copy link

SparkQA commented Feb 8, 2018

Test build #87218 has finished for PR 20548 at commit 367c70b.

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

@gatorsmile
Copy link
Member

This sounds another regression in Spark 2.3. cc @cloud-fan @dilipbiswal

@dilipbiswal
Copy link
Contributor

dilipbiswal commented Feb 13, 2018

@gatorsmile Yeah. Its due to the changes made for SPARK-21759. The fix looks okay to me. One other aspect would be to stop adding the additional projects we keep adding during in IN Coercion.

@cloud-fan
Copy link
Contributor

The fix LGTM. cc @sameeragarwal

@gatorsmile
Copy link
Member

retest this please

@@ -298,22 +298,24 @@ object DataType {
* Returns true if the two data types share the same "shape", i.e. the types (including
* nullability) are the same, but the field names don't need to be the same.
*/
def equalsStructurally(from: DataType, to: DataType): Boolean = {
def equalsStructurally(from: DataType, to: DataType,
ignoreNullability: Boolean = false): Boolean = {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: the indents.

@@ -298,22 +298,24 @@ object DataType {
* Returns true if the two data types share the same "shape", i.e. the types (including
* nullability) are the same, but the field names don't need to be the same.
Copy link
Member

Choose a reason for hiding this comment

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

This comments need an update too.

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87364 has finished for PR 20548 at commit 367c70b.

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

@cloud-fan
Copy link
Contributor

retest this please

@cloud-fan
Copy link
Contributor

Anyone knows which commit introduced this bug?

@dilipbiswal
Copy link
Contributor

dilipbiswal commented Feb 13, 2018

@cloud-fan Its SPARK-21759. Apart from fixing the bug, this PR also refactored the code in checkInputTypes and that seemed to have caused it.

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87381 has finished for PR 20548 at commit 367c70b.

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

asfgit pushed a commit that referenced this pull request Feb 13, 2018
…IN query

## What changes were proposed in this pull request?
Added flag ignoreNullability to DataType.equalsStructurally.
The previous semantic is for ignoreNullability=false.
When ignoreNullability=true equalsStructurally ignores nullability of contained types (map key types, value types, array element types, structure field types).
In.checkInputTypes calls equalsStructurally to check if the children types match. They should match regardless of nullability (which is just a hint), so it is now called with ignoreNullability=true.

## How was this patch tested?
New test in SubquerySuite

Author: Bogdan Raducanu <bogdan@databricks.com>

Closes #20548 from bogdanrdc/SPARK-23316.

(cherry picked from commit 05d0512)
Signed-off-by: gatorsmile <gatorsmile@gmail.com>
@gatorsmile
Copy link
Member

This is a regression introduced by #18968. We have to merge to 2.3. I resolved my comments when I merge the codes.

Thanks! Merged to master/2.3

@asfgit asfgit closed this in 05d0512 Feb 13, 2018
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