Skip to content

Conversation

@cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Mar 9, 2019

What changes were proposed in this pull request?

This PR is a followup of #19559 .

It revisits https://issues.apache.org/jira/browse/SPARK-27117 , which should be an invalid use case according to the SQL standard.

current_date/current_timestamp are reserved keywords, if users want to access columns named current_date/current_timestamp, they should quote the name like select `current_date` from tbl

If ansi mode is not enabled(which is the default), this PR won't introduce any changes.

How was this patch tested?

a new test case

@cloud-fan
Copy link
Contributor Author

UnresolvedAttribute.quoted(ctx.name.getText)
}
}

Copy link
Member

@maropu maropu Mar 9, 2019

Choose a reason for hiding this comment

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

We need to change the current behaviour of select current_date/select current_timestamp in the non-ansi mode? If so, we need to update the migration guide:

// the current behaivour in the non-ansi mode
scala> sql("SET spark.sql.parser.ansi.enabled=false")
scala> sql("SELECT CURRENT_TIMESTAMP").show
+--------------------+                                                          
| current_timestamp()|
+--------------------+
|2019-02-25 16:26:...|
+--------------------+

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, this behavior change only makes sense in ansi mode.

Copy link
Member

Choose a reason for hiding this comment

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

oh, I missed. You're right.

}
}

override def visitCurrentDatetime(ctx: CurrentDatetimeContext): AnyRef = {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be?

override def visitCurrentDatetime(ctx: CurrentDatetimeContext): Expression = withOrigin(ctx) {
  ...
}

@SparkQA
Copy link

SparkQA commented Mar 9, 2019

Test build #103262 has finished for PR 24039 at commit fc765fc.

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

}
}

override def visitCurrentDatetime(ctx: CurrentDatetimeContext): AnyRef = withOrigin(ctx) {
Copy link
Member

Choose a reason for hiding this comment

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

Not AnyRef but Expression?

@SparkQA
Copy link

SparkQA commented Mar 11, 2019

Test build #103297 has finished for PR 24039 at commit 1c0cbdd.

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

@maropu
Copy link
Member

maropu commented Mar 11, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Mar 11, 2019

Test build #103303 has finished for PR 24039 at commit 1c0cbdd.

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

@SparkQA
Copy link

SparkQA commented Mar 11, 2019

Test build #103307 has finished for PR 24039 at commit e014ff0.

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

@cloud-fan
Copy link
Contributor Author

thanks, merging to master!

@cloud-fan cloud-fan closed this in 8114b63 Mar 11, 2019
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.

4 participants