[SPARK-34311][SQL] PostgresDialect can't treat arrays of some types by sarutak · Pull Request #31419 · apache/spark

sarutak · 2021-02-01T13:44:38Z

What changes were proposed in this pull request?

This PR fixes the issue that PostgresDialect can't treat arrays of some types.
Though PostgreSQL supports wide range of types (https://www.postgresql.org/docs/13/datatype.html), the current PostgresDialect can't treat arrays of the following types.

xml
tsvector
tsquery
macaddr
macaddr8
txid_snapshot
pg_snapshot
point
line
lseg
box
path
polygon
circle
pg_lsn
bit varying
interval

NOTE: PostgreSQL doesn't implement arrays of serial types so this PR doesn't care about them.

Why are the changes needed?

To provide better support with PostgreSQL.

Does this PR introduce any user-facing change?

Yes. PostgresDialect can handle arrays of types shown above.

How was this patch tested?

New test.

sarutak · 2021-02-01T14:08:17Z

For reviews: Array of serial is not implemented in PostgreSQL so I didn't care about serial2, serial4 and serial8.

srowen

Seems OK if these new types all have a string representation that's meaningful, and it just helps read more data types.

SparkQA · 2021-02-01T14:46:29Z

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

SparkQA · 2021-02-01T15:22:02Z

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

SparkQA · 2021-02-01T16:36:22Z

Test build #134735 has finished for PR 31419 at commit b0c3ba4.

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

maropu · 2021-02-02T06:52:09Z

...er-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/PostgresIntegrationSuite.scala


    conn.prepareStatement("CREATE TABLE st_with_array (c0 uuid, c1 inet, c2 cidr," +
-      "c3 json, c4 jsonb, c5 uuid[], c6 inet[], c7 cidr[], c8 json[], c9 jsonb[])")
+      "c3 json, c4 jsonb, c5 uuid[], c6 inet[], c7 cidr[], c8 json[], c9 jsonb[], c10 xml[], " +


How about adding tests for non-array cases, e.g., col xml and col tsvector?

I found there are lots of types not tested so far. So I'll open another PR for non-array cases and focus on array cases in this PR.

Does that mean there are types that aren't supported, but for which we support arrays? did I misunderstand that?

I mean that some supported non-array types like numeric, varchar, date and etc are not tested.
So, we need to add tests for those types in addition to the types supported by this PR.

maropu · 2021-02-02T06:55:15Z

Does this PR supports all types in PostgreSQL? are there unsupported types?

sarutak · 2021-02-02T07:33:43Z

Does this PR supports all types in PostgreSQL?

I think so except for serial types as I mentioned above.

Here is all types in PostgreSQL.
https://www.postgresql.org/docs/13/datatype.html

maropu · 2021-02-02T07:38:19Z

okay, could you copy the statement above into the PR description?

sarutak · 2021-02-02T16:04:41Z

I noticed the previous change didn't cover macaddr8, pg_snapshot, bit varying and interval so I've updated the code and description.

SparkQA · 2021-02-02T16:47:44Z

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

SparkQA · 2021-02-02T17:13:53Z

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

SparkQA · 2021-02-02T18:12:15Z

Test build #134788 has finished for PR 31419 at commit ac54aa2.

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

sarutak · 2021-02-02T18:56:32Z

retest this please.

SparkQA · 2021-02-02T19:53:47Z

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

SparkQA · 2021-02-02T20:19:59Z

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

SparkQA · 2021-02-02T21:41:09Z

Test build #134798 has finished for PR 31419 at commit ac54aa2.

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

…s-array-types

SparkQA · 2021-02-08T04:02:25Z

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

SparkQA · 2021-02-08T04:30:56Z

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

SparkQA · 2021-02-08T06:08:21Z

Test build #135000 has finished for PR 31419 at commit 010413e.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class AlterTableSetLocation(
case class AlterTableSetProperties(
case class AlterTableUnsetProperties(
implicit class MetadataColumnHelper(attr: Attribute)
class ResolveSessionCatalog(val catalogManager: CatalogManager)

sarutak · 2021-02-09T16:59:09Z

@srowen @maropu Do you have any comment on the latest change?

srowen

Just so I'm clear, you're saying things like tsvector are already supported, and now tested, and now tsvector[] is also supported? If not then I'm still wondering why the non-array types can't be supported the same way.

sarutak · 2021-02-09T18:27:30Z

Just so I'm clear, you're saying things like tsvector are already supported, and now tested, and now tsvector[] is also supported?

Yes. tsvector is already supported but it's not tested so I opened #31456.
On the other hand, tsvector[] and other array types added in this PR are not supported before this PR.

For non-array types, PostgresDialect maps JDBC OTHER type to StringType.

spark/sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala

Line 41 in 2b51843

Some(StringType)

So, tsvector is already supported.

For array types, PostgresDialect maps the JDBC type of elements to a Spark SQL type according to the type name.
If a corresponding Spark SQL type is defined, JDBC ARRAY is mapped to ArrayType.

spark/sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala

Line 49 in 2b51843

private def toCatalystType(

.
But, if there is no corresponding mapping definition for an array type, it's mapped to null rather than ArrayType.

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala

Line 197 in 2b51843

case java.sql.Types.ARRAY => null

.

sarutak · 2021-02-09T19:51:10Z

@srowen
Ah, you mean how about mapping to StringType by default for array elements?

srowen · 2021-02-09T20:12:50Z

No i just wondered if I should see tests for the non-array types here. It is OK if it's in another PR. This seems fine if you're ready to merge.

maropu

I've checked the test can pass on my local env.

maropu · 2021-02-10T02:29:33Z

Thanks! Merged to master.

sarutak added 2 commits January 28, 2021 15:06

Support XML array type for PostgresDialect.

ae67f42

Support array of some types for PostgresDialect.

b0c3ba4

github-actions bot added the SQL label Feb 1, 2021

srowen approved these changes Feb 1, 2021

View reviewed changes

maropu reviewed Feb 2, 2021

View reviewed changes

Add support for array of macaddr8, interval, varbit and pg_snapshot.

ac54aa2

sarutak mentioned this pull request Feb 3, 2021

[SPARK-34343][SQL][TESTS] Add missing test for some non-array types in PostgreSQL #31456

Closed

Merge branch 'master' of https://github.com/apache/spark into postgre…

010413e

…s-array-types

srowen reviewed Feb 9, 2021

View reviewed changes

maropu approved these changes Feb 10, 2021

View reviewed changes

maropu closed this in f79305a Feb 10, 2021

ulysses-you mentioned this pull request Jun 1, 2023

[SPARK-43267][JDBC] Handle postgres unknown user-defined column as string in array #40953

Closed

Conversation

sarutak commented Feb 1, 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!

sarutak commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 1, 2021

Uh oh!

SparkQA commented Feb 1, 2021

Uh oh!

SparkQA commented Feb 1, 2021

Uh oh!

maropu Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

sarutak Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

srowen Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

sarutak Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

maropu commented Feb 2, 2021

Uh oh!

sarutak commented Feb 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maropu commented Feb 2, 2021

Uh oh!

sarutak commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 2, 2021

Uh oh!

sarutak commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 2, 2021

Uh oh!

SparkQA commented Feb 8, 2021

Uh oh!

SparkQA commented Feb 8, 2021

Uh oh!

SparkQA commented Feb 8, 2021

Uh oh!

sarutak commented Feb 9, 2021

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

sarutak commented Feb 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarutak commented Feb 9, 2021

Uh oh!

srowen commented Feb 9, 2021

Uh oh!

maropu left a comment

Choose a reason for hiding this comment

Uh oh!

maropu commented Feb 10, 2021

Uh oh!

Reviewers

Assignees

sarutak commented Feb 1, 2021 •

edited

Loading

sarutak commented Feb 1, 2021 •

edited

Loading

sarutak commented Feb 2, 2021 •

edited

Loading

sarutak commented Feb 9, 2021 •

edited

Loading