Skip to content

Conversation

@JoshRosen
Copy link
Contributor

@JoshRosen JoshRosen commented Aug 24, 2016

What changes were proposed in this pull request?

When reading float4 and smallint columns from PostgreSQL, Spark's PostgresDialect widens these types to Decimal and Integer rather than using the narrower Float and Short types. According to https://www.postgresql.org/docs/7.1/static/datatype.html#DATATYPE-TABLE, Postgres maps the smallint type to a signed two-byte integer and the real / float4 types to single precision floating point numbers.

This patch fixes this by adding more special-cases to getCatalystType, similar to what was done for the Derby JDBC dialect. I also fixed a similar problem in the write path which causes Spark to create integer columns in Postgres for what should have been ShortType columns.

How was this patch tested?

New test cases in PostgresIntegrationSuite (which I ran manually because Jenkins can't run it right now).

@SparkQA
Copy link

SparkQA commented Aug 24, 2016

Test build #64375 has finished for PR 14796 at commit 5c2b84e.

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

@SparkQA
Copy link

SparkQA commented Aug 24, 2016

Test build #64376 has finished for PR 14796 at commit bc4dee3.

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

case FloatType => Some(JdbcType("FLOAT4", Types.FLOAT))
case DoubleType => Some(JdbcType("FLOAT8", Types.DOUBLE))
case ShortType => Some(JdbcType("SMALLINT", java.sql.Types.SMALLINT))
case t: DecimalType => Some(
Copy link
Member

Choose a reason for hiding this comment

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

Hi @JoshRosen, do you mind if I ask a question please (although it is a super minor)? It seems java.sql.Types.SMALLINT and Types.SMALLINT are pointing the same constant. Are there a reason to do this like this?

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, no good reason; just my IDE autocomplete acting up. Lemme fix it.

@SparkQA
Copy link

SparkQA commented Aug 25, 2016

Test build #64402 has finished for PR 14796 at commit 708343d.

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

@hvanhovell
Copy link
Contributor

LGTM - merging to master. Thanks!

@asfgit asfgit closed this in a133057 Aug 25, 2016
@JoshRosen JoshRosen deleted the postgres-jdbc-type-fixes branch August 25, 2016 21:58
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