[SPARK-35017][SQL] Transfer ANSI intervals via Hive Thrift server #32121

MaxGekk · 2021-04-10T09:53:14Z

What changes were proposed in this pull request?

Map Catalyst's interval types to Hive's types:
- YearMonthIntervalType -> interval_year_month
- DayTimeIntervalType -> interval_day_time
Invoke HiveResult.toHiveString() to convert external intervals types java.time.Period/java.time.Duration to strings.

Why are the changes needed?

To be able to retrieve ANSI intervals via Hive Thrift server.
This fixes the issue:

 $ ./sbin/start-thriftserver.sh
 $ ./bin/beeline
Beeline version 2.3.8 by Apache Hive
beeline> !connect jdbc:hive2://localhost:10000/default "" "" ""
Connecting to jdbc:hive2://localhost:10000/default
Connected to: Spark SQL (version 3.2.0-SNAPSHOT)
0: jdbc:hive2://localhost:10000/default> select timestamp'2021-01-01 01:02:03.000001' - date'2020-12-31';
Error: java.lang.IllegalArgumentException: Unrecognized type name: day-time interval (state=,code=0)

It should unblock [SPARK-34905][SQL][TESTS] Enable ANSI intervals in SQLQueryTestSuite/ThriftServerQueryTestSuite #32099 which enables *.sql tests in ThriftServerQueryTestSuite.

Does this PR introduce any user-facing change?

Yes. After the changes:

0: jdbc:hive2://localhost:10000/default> select timestamp'2021-01-01 01:02:03.000001' - date'2020-12-31';
+----------------------------------------------------+
| subtracttimestamps(TIMESTAMP '2021-01-01 01:02:03.000001', DATE '2020-12-31') |
+----------------------------------------------------+
| 1 01:02:03.000001000                               |
+----------------------------------------------------+
1 row selected (1.637 seconds)

How was this patch tested?

By running new test:

$ ./build/sbt -Phive -Phive-thriftserver "test:testOnly *SparkThriftServerProtocolVersionsSuite"
$ ./build/sbt -Phive -Phive-thriftserver "test:testOnly *SparkMetadataOperationSuite"

Also checked an array of an interval:

0: jdbc:hive2://localhost:10000/default> select array(timestamp'2021-01-01 01:02:03.000001' - date'2020-12-31');
+----------------------------------------------------+
| array(subtracttimestamps(TIMESTAMP '2021-01-01 01:02:03.000001', DATE '2020-12-31')) |
+----------------------------------------------------+
| [1 01:02:03.000001000]                             |
+----------------------------------------------------+

MaxGekk · 2021-04-10T10:00:12Z

...st/scala/org/apache/spark/sql/hive/thriftserver/SparkThriftServerProtocolVersionsSuite.scala

      }
    }
+
+    test(s"SPARK-35017: $version get day-time interval type") {


I tested only day-time interval because constructing of year-month interval in SQL is not possible at the moment. I created the JIRA for that: SPARK-35018

The PR #32240 adds a test for year-month intervals.

SparkQA · 2021-04-10T11:00:16Z

Test build #137168 has finished for PR 32121 at commit 1c6c93d.

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

SparkQA · 2021-04-10T11:32:07Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41746/

SparkQA · 2021-04-10T11:32:08Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41746/

yaooqinn · 2021-04-11T02:31:47Z

...r/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkExecuteStatementOperation.scala

          timeFormatters)
      case _: ArrayType | _: StructType | _: MapType | _: UserDefinedType[_] =>
        to += toHiveString((from.get(ordinal), dataTypes(ordinal)), false, timeFormatters)
+      case YearMonthIntervalType =>


How about nested types?

If you mean the array, map types, they are converted to strings and they don't have a structure that could be recognized by Hive lib. Don't think I can do something here in the PR. This is a restriction of current implementation, and it should be solved in general way.

For example, after the PR #32120 is merged:

0: jdbc:hive2://localhost:10000/default> select array(timestamp'2021-01-01 01:02:03.000001' - date'2020-12-31'); +----------------------------------------------------+ | array(subtracttimestamps(TIMESTAMP '2021-01-01 01:02:03.000001', DATE '2020-12-31')) | +----------------------------------------------------+ | [1 01:02:03.000001000] | +----------------------------------------------------+

I mean the client-side get the plain YearMonthIntervalType/DayTimeIntervalType values via HiveIntervalYearMonth/DayTimeIntervalType.toString, but the ones that in nested types are handled in toHiveString. They look the same but these branches seem to be redundant and may cause inconsistency if the underlying hive changed

Is

case YearMonthIntervalType | DayTimeIntervalType | _: ArrayType | _: StructType | _: MapType | _: UserDefinedType[_] =>

better as we handle them consistently at spark-side?

If I do as you propose, I get the exception on my test:

[info] Cause: java.lang.ClassCastException: java.lang.String cannot be cast to org.apache.hadoop.hive.common.type.HiveIntervalDayTime [info] at org.apache.hive.service.cli.ColumnValue.toTColumnValue(ColumnValue.java:174) [info] at org.apache.hive.service.cli.RowBasedSet.addRow(RowBasedSet.java:60) [info] at org.apache.hive.service.cli.RowBasedSet.addRow(RowBasedSet.java:32)

at

case INTERVAL_YEAR_MONTH_TYPE: return stringValue((HiveIntervalYearMonth) value); case INTERVAL_DAY_TIME_TYPE: return stringValue((HiveIntervalDayTime) value);

Probably, I could just replace the casting by string casting. Let me try.

…ft-protocol

wangyum · 2021-04-11T13:10:07Z

...r/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkExecuteStatementOperation.scala

 import scala.collection.mutable.ArrayBuffer
 import scala.util.control.NonFatal

+import org.apache.hadoop.hive.common.`type`.{HiveIntervalDayTime, HiveIntervalYearMonth}


Add HiveIntervalDayTime and HiveIntervalYearMonth to

spark/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkGetTypeInfoOperation.scala

Lines 97 to 103 in d7dc42d

val supportedType: Seq[Type] = {

Seq(NULL_TYPE, BOOLEAN_TYPE, STRING_TYPE, BINARY_TYPE,

TINYINT_TYPE, SMALLINT_TYPE, INT_TYPE, BIGINT_TYPE,

FLOAT_TYPE, DOUBLE_TYPE, DECIMAL_TYPE,

DATE_TYPE, TIMESTAMP_TYPE,

ARRAY_TYPE, MAP_TYPE, STRUCT_TYPE, CHAR_TYPE, VARCHAR_TYPE)

}

?

SparkQA · 2021-04-11T17:02:12Z

Test build #137181 has finished for PR 32121 at commit feeeea5.

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

SparkQA · 2021-04-11T17:06:16Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41759/

yaooqinn

LGTM

HyukjinKwon

Looks making sense to me.

HyukjinKwon · 2021-04-12T02:56:05Z

Merged to master.

cloud-fan · 2021-04-12T05:44:32Z

late LGTM

Transfer ANSI intervals via Hive Thrift server

1c6c93d

github-actions bot added the SQL label Apr 10, 2021

MaxGekk commented Apr 10, 2021

View reviewed changes

MaxGekk mentioned this pull request Apr 10, 2021

[SPARK-34905][SQL][TESTS] Enable ANSI intervals in SQLQueryTestSuite/ThriftServerQueryTestSuite #32099

Closed

yaooqinn reviewed Apr 11, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into ansi-intervals-thri…

918170f

…ft-protocol

MaxGekk requested review from HyukjinKwon, cloud-fan, wangyum and yaooqinn April 11, 2021 09:36

wangyum reviewed Apr 11, 2021

View reviewed changes

MaxGekk added 2 commits April 11, 2021 18:40

Serialize ANSI intervals as strings

26a6235

Extend supportedType

feeeea5

wangyum approved these changes Apr 12, 2021

View reviewed changes

yaooqinn approved these changes Apr 12, 2021

View reviewed changes

HyukjinKwon approved these changes Apr 12, 2021

View reviewed changes

HyukjinKwon closed this in 90820b3 Apr 12, 2021

MaxGekk mentioned this pull request Apr 19, 2021

[SPARK-35018][SQL][TESTS] Check transferring of year-month intervals via Hive Thrift server #32240

Closed

MaxGekk mentioned this pull request Apr 27, 2021

[SPARK-35085][SQL] Get columns operation should handle ANSI interval column properly #32345

Closed

	val supportedType: Seq[Type] = {
	Seq(NULL_TYPE, BOOLEAN_TYPE, STRING_TYPE, BINARY_TYPE,
	TINYINT_TYPE, SMALLINT_TYPE, INT_TYPE, BIGINT_TYPE,
	FLOAT_TYPE, DOUBLE_TYPE, DECIMAL_TYPE,
	DATE_TYPE, TIMESTAMP_TYPE,
	ARRAY_TYPE, MAP_TYPE, STRUCT_TYPE, CHAR_TYPE, VARCHAR_TYPE)
	}

[SPARK-35017][SQL] Transfer ANSI intervals via Hive Thrift server #32121

[SPARK-35017][SQL] Transfer ANSI intervals via Hive Thrift server #32121

Uh oh!

Conversation

MaxGekk commented Apr 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 10, 2021

Uh oh!

SparkQA commented Apr 10, 2021

Uh oh!

SparkQA commented Apr 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 11, 2021

Uh oh!

SparkQA commented Apr 11, 2021

Uh oh!

yaooqinn left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Apr 12, 2021

Uh oh!

cloud-fan commented Apr 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

MaxGekk commented Apr 10, 2021 •

edited

Loading