[WIP][SPARK-29155][SQL] Support special date/timestamp values in the PostgreSQL dialect only by MaxGekk · Pull Request #25834 · apache/spark

MaxGekk · 2019-09-18T18:14:50Z

What changes were proposed in this pull request?

In the PR, I propose to support special timestamp and date values introduced by #25716 and #25708 only when the SQL config spark.sql.dialect is set to PostgreSQL.

Why are the changes needed?

The special values are PostgreSQL specific feature. It should be supported under a config to avoid performance penalties and potential impact on user apps.

Does this PR introduce any user-facing change?

Yes, special date/timestamp values like epoch, now, today, tomorrow, yesterday are supported only in the PostgreSQL dialect.

In the Spark dialect (the default dialect):

spark-sql>setspark.sql.dialect=Spark;
spark.sql.dialect	Spark
spark-sql>selecttimestamp'today';
Error in query: Cannot parse the TIMESTAMP value: today(line1, pos 7)
== SQL ==selecttimestamp'today'-------^^^

In the PostgreSQL dialect:

spark-sql>setspark.sql.dialect=PostgreSQL;
spark.sql.dialect	PostgreSQL
spark-sql>selecttimestamp'today';
2019-09-1800:00:00

How was this patch tested?

Updated existing test suite TimestampFormatterSuite, DateTimeUtilsSuite and Csv/JsonFunctionsSuite.

MaxGekk · 2019-09-18T18:18:24Z

@HyukjinKwon This is the draft PR for #25716 (review) . I added the config from #25697 . As soon as this PR #25708 for dates be merged, I will put special date values under the config as well.

SparkQA · 2019-09-18T23:14:15Z

Test build #110929 has finished for PR 25834 at commit 2d47906.

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

…ues-under-config # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala # sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

SparkQA · 2019-09-19T07:05:01Z

Test build #110973 has finished for PR 25834 at commit b8ed08b.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
public class ExecutorPluginContext
class ExecutorPluginSource(name: String) extends Source
class QuantileDiscretizer(JavaEstimator, HasInputCol, HasOutputCol, HasInputCols, HasOutputCols,
implicit class MultipartIdentifierHelper(parts: Seq[String])

MaxGekk · 2019-09-19T07:37:20Z

jenkins, retest this, please

SparkQA · 2019-09-19T12:14:27Z

Test build #110983 has finished for PR 25834 at commit b8ed08b.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
public class ExecutorPluginContext
class ExecutorPluginSource(name: String) extends Source
class QuantileDiscretizer(JavaEstimator, HasInputCol, HasOutputCol, HasInputCols, HasOutputCols,
implicit class MultipartIdentifierHelper(parts: Seq[String])

…ues-under-config

HyukjinKwon · 2019-09-22T10:14:17Z

cc @maropu and @cloud-fan

SparkQA · 2019-09-22T12:54:48Z

Test build #111149 has finished for PR 25834 at commit 3227968.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class JavaClassifierParams(HasRawPredictionCol, JavaPredictorParams):
class JavaClassifier(JavaPredictor, JavaClassifierParams):
class JavaClassificationModel(JavaPredictionModel, JavaClassifierParams):
class JavaProbabilisticClassifierParams(HasProbabilityCol, HasThresholds, JavaClassifierParams):
class JavaProbabilisticClassifier(JavaClassifier, JavaProbabilisticClassifierParams):
class JavaProbabilisticClassificationModel(JavaClassificationModel,
class LinearSVC(JavaClassifier, HasMaxIter, HasRegParam, HasTol,
class LinearSVCModel(JavaClassificationModel, JavaMLWritable, JavaMLReadable):
class LogisticRegression(JavaProbabilisticClassifier, HasMaxIter, HasRegParam, HasTol,
class LogisticRegressionModel(JavaProbabilisticClassificationModel, JavaMLWritable, JavaMLReadable,
class DecisionTreeClassifier(JavaProbabilisticClassifier, HasWeightCol,
class DecisionTreeClassificationModel(DecisionTreeModel, JavaProbabilisticClassificationModel,
class RandomForestClassifier(JavaProbabilisticClassifier, HasSeed, RandomForestParams,
class RandomForestClassificationModel(TreeEnsembleModel, JavaProbabilisticClassificationModel,
class GBTClassifier(JavaProbabilisticClassifier, GBTClassifierParams, HasCheckpointInterval,
class GBTClassificationModel(TreeEnsembleModel, JavaProbabilisticClassificationModel,
class NaiveBayes(JavaProbabilisticClassifier, HasThresholds, HasWeightCol,
class NaiveBayesModel(JavaProbabilisticClassificationModel, JavaMLWritable, JavaMLReadable):
class MultilayerPerceptronClassifier(JavaProbabilisticClassifier, HasMaxIter, HasTol, HasSeed,
class MultilayerPerceptronClassificationModel(JavaProbabilisticClassificationModel, JavaMLWritable,
class OneVsRestParams(JavaClassifierParams, HasWeightCol):
class LinearRegression(JavaPredictor, HasMaxIter, HasRegParam, HasTol, HasElasticNetParam,
class LinearRegressionModel(JavaPredictionModel, GeneralJavaMLWritable, JavaMLReadable,
class IsotonicRegression(JavaPredictor, HasWeightCol, JavaMLWritable, JavaMLReadable):
class IsotonicRegressionModel(JavaPredictionModel, JavaMLWritable, JavaMLReadable):
class DecisionTreeRegressor(JavaPredictor, HasWeightCol, DecisionTreeParams, TreeRegressorParams,
class DecisionTreeModel(JavaPredictionModel):
class RandomForestRegressor(JavaPredictor, HasSeed, RandomForestParams, TreeRegressorParams,
class GBTRegressor(JavaPredictor, GBTRegressorParams, HasCheckpointInterval, HasSeed,
class AFTSurvivalRegression(JavaPredictor, HasFitIntercept, HasMaxIter, HasTol,
class AFTSurvivalRegressionModel(JavaPredictionModel, JavaMLWritable, JavaMLReadable):
class GeneralizedLinearRegression(JavaPredictor, HasFitIntercept, HasMaxIter, HasTol, HasRegParam,
class GeneralizedLinearRegressionModel(JavaPredictionModel, JavaMLWritable,
class JavaPredictorParams(HasLabelCol, HasFeaturesCol, HasPredictionCol):
class JavaPredictor(JavaEstimator, JavaPredictorParams):
class JavaPredictionModel(JavaModel, JavaPredictorParams):
abstract class PartitionTransformExpression extends Expression with Unevaluable
case class Years(child: Expression) extends PartitionTransformExpression
case class Months(child: Expression) extends PartitionTransformExpression
case class Days(child: Expression) extends PartitionTransformExpression
case class Hours(child: Expression) extends PartitionTransformExpression
case class Bucket(numBuckets: Literal, child: Expression) extends PartitionTransformExpression
implicit class OptionsHelper(options: Map[String, String])
trait WriteConfigMethods[R]
trait CreateTableWriter[T] extends WriteConfigMethods[CreateTableWriter[T]]

…UnsafeArraySuite

SparkQA · 2019-09-22T19:49:06Z

Test build #111164 has finished for PR 25834 at commit 4f35fad.

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

SparkQA · 2019-09-22T20:06:46Z

Test build #111165 has finished for PR 25834 at commit 2a86d9c.

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

maropu · 2019-09-23T00:53:53Z

Hi, @MaxGekk , thanks for the work! one question: don't we need some summary documents for these PgSQL-specific behaviour? If we add more and more features for this flag in future, it might be difficult for users to follow them. cc: @gatorsmile @dongjoon-hyun

cloud-fan · 2019-09-23T02:59:09Z

We need a document page to explain the pgsql dialect. There will be a lot of things and it's too much to put in the SQLConf. Also cc @gengliangwang

MaxGekk · 2019-09-23T05:17:52Z

Any ideas where I can create such page about supported features of PostgreSQL dialect?

SparkQA · 2019-09-23T07:05:02Z

Test build #111188 has finished for PR 25834 at commit ea42053.

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

SparkQA · 2019-09-23T07:05:02Z

Test build #111189 has finished for PR 25834 at commit 53afcdb.

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

MaxGekk · 2019-09-23T07:29:16Z

jenkins, retest this, please

SparkQA · 2019-09-23T09:23:43Z

Test build #111199 has finished for PR 25834 at commit 53afcdb.

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

SparkQA · 2019-09-23T16:44:00Z

Test build #111221 has finished for PR 25834 at commit 94abaea.

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

cloud-fan · 2019-09-25T03:10:28Z

With hindsight, I think this feature (special datetime strings) doesn't conflict with Spark and seems very intuitive. Do we really need to control it by a flag?

MaxGekk · 2019-09-25T09:53:31Z

Do we really need to control it by a flag?

I would keep the feature without any flags. For me it makes sense to hide it only if the feature brings any performance penalty but it should not (I do believe but we can recheck this) because condition for branching should be cheap.

cloud-fan · 2019-09-25T14:44:19Z

Even if pgsql doesn't have this feature, I would accept this feature as an improvement to Spark. Shall we simply add a migration guide instead of protecting it via a flag? cc @gatorsmile @gengliangwang

gengliangwang · 2019-09-25T15:51:25Z

This feature doesn't conflict with the existing behavior of Spark. I think we don't need the dialect flag here.

maropu · 2019-09-26T06:14:11Z

I think we need a consistent rule for that... for example, the case 2 in #25697 is conflict with the existing behaivours?

gengliangwang · 2019-09-26T07:15:00Z

the case 2 in #25697 is conflict with the existing behaivours?

I think so. Previously the result of cast('tru' as boolean) is not true in Spark.

MaxGekk · 2019-09-26T15:51:08Z

Please, let me know what I should do next 1. either rebase this PR or 2. open another one for updating the SQL migration guide.

cloud-fan · 2019-09-26T15:58:18Z

I prefer to do 2. This is a nice feature to have in Spark. cast('tru' as boolean) is different as it's less intuitive and looks like pgsql-specific.

MaxGekk · 2019-09-27T12:42:09Z

Please, take a look at #25948

MaxGekk added 6 commits September 18, 2019 21:31

Add the config: spark.sql.dialect
d234408

Support the config in TimestampFormatter
676d265

Add isPostgreSqlDialect to SQLConf
e434d3c

Support the config by stringToTimestamp
ec75350

Fix Json/CsvFunctionsSuite
3cd837f

Enable the PostgreSQL dialect for PostgreSQL tests
2d47906

dongjoon-hyun added the SQL label Sep 19, 2019

Merge remote-tracking branch 'remotes/origin/master' into special-val…
b8ed08b
…ues-under-config # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala # sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

Merge remote-tracking branch 'remotes/origin/master' into special-val…
3227968
…ues-under-config

MaxGekk added 4 commits September 22, 2019 20:59

Support the flag in DateFormatter
bbdd65f

Support the flag in stringToDate
f90df73

Enable supportSpecialValues by default in DateTimeUtilsSuite
4f35fad

Enable special timestamp values by default in DateTimeUtilsSuite and …
2a86d9c
…UnsafeArraySuite

This was referenced Sep 22, 2019
[SPARK-29012][SQL] Support special timestamp values #25716
Closed
[SPARK-28141][SQL] Support special date values #25708
Closed

maropu reviewed Sep 23, 2019
View reviewed changes

Comment threadsql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala

MaxGekk added 3 commits September 23, 2019 09:40

Fix CsvFunctionsSuite
b340f5e

Fix JsonFunctionsSuite
fd34eb6

Pass boolean literal directly in Cast
ba6fc7b

Pass boolean literal for date directly in Cast
ea42053

Enable spacial values in HashExpressionsSuite by default
53afcdb

Fix CastSuite for the date case
94abaea

MaxGekk mentioned this pull request Sep 27, 2019
[SPARK-29275][SQL][DOC] Describe special date/timestamp values in the SQL migration guide #25948
Closed

dongjoon-hyun closed this in 4bffcf5Sep 27, 2019

MaxGekk deleted the special-values-under-config branch October 15, 2019 19:57

Uh oh!

Conversation

MaxGekk commented Sep 18, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

MaxGekk commented Sep 18, 2019

Uh oh!

SparkQA commented Sep 18, 2019

Uh oh!

SparkQA commented Sep 19, 2019

Uh oh!

MaxGekk commented Sep 19, 2019

Uh oh!

SparkQA commented Sep 19, 2019

Uh oh!

HyukjinKwon commented Sep 22, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Sep 22, 2019

Uh oh!

SparkQA commented Sep 22, 2019

Uh oh!

SparkQA commented Sep 22, 2019

Uh oh!

maropu commented Sep 23, 2019

Uh oh!

Uh oh!

cloud-fan commented Sep 23, 2019

Uh oh!

MaxGekk commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

MaxGekk commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

cloud-fan commented Sep 25, 2019

Uh oh!

MaxGekk commented Sep 25, 2019

Uh oh!

cloud-fan commented Sep 25, 2019

Uh oh!

gengliangwang commented Sep 25, 2019

Uh oh!

maropu commented Sep 26, 2019

Uh oh!

gengliangwang commented Sep 26, 2019

Uh oh!

MaxGekk commented Sep 26, 2019

Uh oh!

cloud-fan commented Sep 26, 2019

Uh oh!

MaxGekk commented Sep 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

HyukjinKwon commented Sep 22, 2019•
edited
Loading