Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Nov 1, 2019

What changes were proposed in this pull request?

Currently we only support year[s], month[s] etc to define interval literals.. this pr support its common abbreviations to define.

"year" | "years" | "y" | "yr" | "yrs" => YEAR
"month" | "months" | "mon" | "mons" => MONTH
"week" | "weeks" | "w" => WEEK
"day" | "days" | "d" => DAY
"hour" | "hours" | "h" | "hr" | "hrs" => HOUR
"minute" | "minutes" | "m" | "min" | "mins" => MINUTE
"second" | "seconds" | "s" | "sec" | "secs" => SECOND
"millisecond" | "milliseconds" | "ms" | "msec" | "msecs" | "mseconds" => MILLISECOND
"microsecond" | "microseconds" | "us" | "usec" | "usecs" | "useconds" => MICROSECOND

Why are the changes needed?

Postgres Feature Parity

postgres=# select interval '1 yrs 2 hrs';
    interval
-----------------
 1 year 02:00:00
(1 row)

Does this PR introduce any user-facing change?

yes, before we only can define interval literals in

select interval '1 years 2 months 3 weeks 4 days 5 hours 6 minutes 7 seconds 8 milliseconds 9 microseconds';

now we can use abbreviations of the units

select interval '1 y 2 mon 3 w 4 d 5 h 6 m 7 s 8 ms 9 us';

How was this patch tested?

ADD UT

@SparkQA
Copy link

SparkQA commented Nov 1, 2019

Test build #113087 has finished for PR 26359 at commit 91c2ee2.

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

@SparkQA
Copy link

SparkQA commented Nov 1, 2019

Test build #113101 has finished for PR 26359 at commit 4088f88.

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

@SparkQA
Copy link

SparkQA commented Nov 1, 2019

Test build #113105 has finished for PR 26359 at commit 838ed77.

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

DATABASES: 'DATABASES' | 'SCHEMAS';
DAY: 'DAY';
DAYS: 'DAYS';
DAYS: 'DAYS'| 'D';
Copy link
Contributor

Choose a reason for hiding this comment

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

D is not even a keyword in pgsql: https://www.postgresql.org/docs/8.1/sql-keywords-appendix.html

I'm curious how pgsql supports these abbreviations.

Copy link
Member Author

@yaooqinn yaooqinn Nov 4, 2019

Choose a reason for hiding this comment

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

yes, not abbreviation but also its plural

Copy link
Member Author

@yaooqinn yaooqinn Nov 4, 2019

Choose a reason for hiding this comment

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

It seems that in pg the type_name interval is required

Copy link
Member Author

@yaooqinn yaooqinn Nov 4, 2019

Choose a reason for hiding this comment

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

postgres=# select  '1 s 2 days 6 weeks 3 seconds';
           ?column?
------------------------------
 1 s 2 days 6 weeks 3 seconds
(1 row)

postgres=# select  '1 s 2 days 6 weeks';
      ?column?
--------------------
 1 s 2 days 6 weeks
(1 row)

postgres=# select  interval '1 s 2 days 6 weeks';
     interval
------------------
 44 days 00:00:01
(1 row)

postgres=# select  interval '1 s 2 days 3 weeks';
     interval
------------------
 23 days 00:00:01
(1 row)

postgres=# select  interval '1 s 2 days d weeks';
    interval
-----------------
 2 days 00:00:01
(1 row)

postgres=# select  interval '1 s 2 days days weeks';
    interval
-----------------
 2 days 00:00:01
(1 row)

postgres=# select  interval '1 s 2 days year weeks';
    interval
-----------------
 2 days 00:00:01
(1 row)

postgres=# select  interval '1 s 2 yrs days weeks';
     interval
------------------
 2 years 00:00:01

postgres=# select  interval '1s 2yrs days weeks';
     interval
------------------
 2 years 00:00:01
(1 row)
(1 row)

Copy link
Member Author

Choose a reason for hiding this comment

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

proposed a new approach to parse intervals in the form of abbreviations, without adding new keywords to parser, please take a look, thanks.

@SparkQA
Copy link

SparkQA commented Nov 4, 2019

Test build #113194 has finished for PR 26359 at commit 4e27e36.

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

@SparkQA
Copy link

SparkQA commented Nov 4, 2019

Test build #113200 has finished for PR 26359 at commit fc5843f.

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

@SparkQA
Copy link

SparkQA commented Nov 8, 2019

Test build #113453 has finished for PR 26359 at commit 278ecd8.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • public class DateTimeConstants
  • .doc(\"Comma-separated list of class names implementing \" +
  • sealed abstract class PluginContainer
  • class PluginMetricsSource(
  • case class PluginMessage(pluginName: String, message: AnyRef)
  • abstract class IntervalNumOperation(
  • case class MultiplyInterval(interval: Expression, num: Expression)
  • case class DivideInterval(interval: Expression, num: Expression)
  • case class AlterTableAddPartitionStatement(
  • case class AlterTableSerDePropertiesStatement(
  • case class ShowCurrentNamespaceStatement() extends ParsedStatement
  • case class ShowCurrentNamespace(catalogManager: CatalogManager) extends Command
  • case class LocalShuffleReaderExec(child: SparkPlan) extends UnaryExecNode
  • case class ShowCurrentNamespaceExec(
  • class ContinuousRecordEndpoint(buckets: Seq[Seq[UnsafeRow]], lock: Object)

@SparkQA
Copy link

SparkQA commented Nov 9, 2019

Test build #113496 has finished for PR 26359 at commit 6e0e4db.

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

@SparkQA
Copy link

SparkQA commented Nov 9, 2019

Test build #113497 has finished for PR 26359 at commit aba538e.

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

@SparkQA
Copy link

SparkQA commented Nov 9, 2019

Test build #113498 has finished for PR 26359 at commit b737404.

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

@SparkQA
Copy link

SparkQA commented Nov 9, 2019

Test build #113500 has finished for PR 26359 at commit c4f7bf3.

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

@SparkQA
Copy link

SparkQA commented Nov 10, 2019

Test build #113537 has finished for PR 26359 at commit 3736e70.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

/**
* A safe version of `fromString`. It returns null for invalid input string.
*/
def safeFromString(str: String): CalendarInterval = {
Copy link
Member Author

@yaooqinn yaooqinn Nov 10, 2019

Choose a reason for hiding this comment

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

This method is removed because never used

@SparkQA
Copy link

SparkQA commented Nov 10, 2019

Test build #113539 has finished for PR 26359 at commit dff647e.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 10, 2019

Test build #113541 has finished for PR 26359 at commit a37f3b2.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 10, 2019

Test build #113542 has finished for PR 26359 at commit 899eb0c.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 10, 2019

Test build #113543 has finished for PR 26359 at commit 1b1ccb8.

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

@SparkQA
Copy link

SparkQA commented Nov 11, 2019

Test build #113560 has finished for PR 26359 at commit 116d92e.

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

@SparkQA
Copy link

SparkQA commented Nov 11, 2019

Test build #113564 has finished for PR 26359 at commit b75dbdb.

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

@yaooqinn yaooqinn requested a review from cloud-fan November 11, 2019 07:42
@yaooqinn
Copy link
Member Author

Without adding extra keywords to parser to support abbreviations and plurals, we need to directly parse the interval string instead of delegating it to the catalyst parser again.

Features pending on this:

  1. support sql standard interval input as typed literals, i.e.
postgres=# select interval '1-10 5 1:1:1';
            interval
--------------------------------
 1 year 10 mons 5 days 01:01:01
(1 row)
  1. support abbreviation format in casting to strings

gently ping @cloud-fan again.

@yaooqinn yaooqinn closed this Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants