Skip to content

Conversation

@cloud-fan
Copy link
Contributor

Add ARRAY support to PostgresDialect.

Nested ARRAY is not allowed for now because it's hard to get the array dimension info. See http://stackoverflow.com/questions/16619113/how-to-get-array-base-type-in-postgres-via-jdbc

Thanks for the initial work from @mariusvniekerk !

Close #9137

@cloud-fan
Copy link
Contributor Author

cc @marmbrus

@SparkQA
Copy link

SparkQA commented Nov 12, 2015

Test build #45737 has finished for PR 9662 at commit b7db9ec.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class ArrayConversion(elementConversion: JDBCConversion) extends JDBCConversion\n

@mariusvniekerk
Copy link
Member

I've added write support in #9137 as well if you want to just use it from there.

@SparkQA
Copy link

SparkQA commented Nov 12, 2015

Test build #45739 has finished for PR 9662 at commit 378c5a9.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class ArrayConversion(elementConversion: JDBCConversion) extends JDBCConversion\n

Copy link
Member

Choose a reason for hiding this comment

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

Is that the same in all backends that support arrays (Oracle etc)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, it's not always working

@SparkQA
Copy link

SparkQA commented Nov 12, 2015

Test build #45742 has finished for PR 9662 at commit fbaa543.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class ArrayConversion(elementConversion: JDBCConversion) extends JDBCConversion\n * abstract class JdbcDialect extends Serializable\n

@mariusvniekerk
Copy link
Member

These test failures don't seem to be related?

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45794 has finished for PR 9662 at commit fbaa543.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class ArrayConversion(elementConversion: JDBCConversion) extends JDBCConversion\n * abstract class JdbcDialect extends Serializable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a public API, if we add fields here we are going to break any existing implementations. Do we have to do this?

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46050 has finished for PR 9662 at commit ad52183.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class ArrayConversion(elementConversion: JDBCConversion) extends JDBCConversion\n * abstract class JdbcDialect extends Serializable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just Literal.create(null, a.dataType)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yea, we can simply this.

@marmbrus
Copy link
Contributor

Thanks, I'm going to merge this into master and 1.6.

asfgit pushed a commit that referenced this pull request Nov 17, 2015
Add ARRAY support to `PostgresDialect`.

Nested ARRAY is not allowed for now because it's hard to get the array dimension info. See http://stackoverflow.com/questions/16619113/how-to-get-array-base-type-in-postgres-via-jdbc

Thanks for the initial work from mariusvniekerk !

Close #9137

Author: Wenchen Fan <wenchen@databricks.com>

Closes #9662 from cloud-fan/postgre.

(cherry picked from commit d925149)
Signed-off-by: Michael Armbrust <michael@databricks.com>
@asfgit asfgit closed this in d925149 Nov 17, 2015
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.

4 participants