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

Add datetime literal printing for Redshift, Trino, and Spark #53

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

alancai98
Copy link
Member

@alancai98 alancai98 commented Jun 25, 2024

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@alancai98 alancai98 self-assigned this Jun 25, 2024
@alancai98
Copy link
Member Author

alancai98 commented Jun 25, 2024

From testing some observations:

  • Redshift TIME + TIMESTAMP with an offset but not using WITH TIME ZONE removes the offset in the output
  • Trino TIME + TIMESTAMP with an offset but not using WITH TIME ZONE keeps the offset in the output
  • Trino does not support WITH TIME ZONE syntax
  • Spark does not have a TIME literal -> think we'll need to use timestamp instead should error?

@alancai98 alancai98 marked this pull request as ready for review June 25, 2024 23:44
Comment on lines +55 to +56
* TODO: Copied from partiql-lang-kotlin's [org.partiql.value.io.PartiQLValueTextWriter]. Delete after
* https://github.com/partiql/partiql-lang-kotlin/issues/1491 is resolved and released as a new PartiQL Kotlin version.
Copy link
Member Author

Choose a reason for hiding this comment

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

File is identical to the PartiQLValueWriter from partiql/partiql-lang-kotlin#1492. Copied here until we release the updated PartiQLValueWriter in a new release.

@@ -33,7 +33,6 @@ import org.partiql.ast.visitor.AstBaseVisitor
import org.partiql.value.MissingValue
import org.partiql.value.NullValue
import org.partiql.value.PartiQLValueExperimental
import org.partiql.value.io.PartiQLValueTextWriter
Copy link
Member Author

Choose a reason for hiding this comment

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

Have the base SqlDialect rely on the Scribe PartiQLValueTextWriter rather than the v0.14.5 PartiQLValueTextWriter, which is missing the latest date, time, timestamp printing.

Comment on lines +87 to +89
// Essentially the same as `SqlDialect`. For SQL time and timestamp literals, Redshift (like Postgresql) requires
// specifying if the time/timestamp has a timezone in the type name. Otherwise, the offset will be ignored.
// Postgresql reference -- https://www.postgresql.org/docs/16/datatype-datetime.html#DATATYPE-DATETIME-INPUT-TIME-STAMPS
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment summarizes the changes for Redshift time and timestamp w/ offset translation. Date remains the same.

@@ -113,6 +114,15 @@ public open class SparkDialect : SqlDialect() {
return tail concat list(start, end) { node.values }
}

@OptIn(PartiQLValueExperimental::class)
Copy link
Member Author

Choose a reason for hiding this comment

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

Spark date and timestamp literals follow same behavior as PartiQL. Spark does not have a time type so error.

@@ -0,0 +1,48 @@
-- DATE
Copy link
Member Author

Choose a reason for hiding this comment

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

Fortunately, Trino seems to have the same parsing and evaluation behavior as PartiQL for datetime types.

@alancai98 alancai98 requested a review from rchowell June 25, 2024 23:49
@alancai98 alancai98 changed the title Add temporary datetime literal printing (TODO add some testing) Add temporary datetime literal printing Jun 25, 2024
@alancai98 alancai98 changed the title Add temporary datetime literal printing Add datetime literal printing for Redshift, Trino, and Spark Jun 25, 2024
@rchowell rchowell merged commit 22925e1 into main Jun 26, 2024
@rchowell rchowell deleted the add-datetime-lit-printing branch June 26, 2024 16:48
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.

2 participants