Skip to content

Conversation

@wangyum
Copy link
Member

@wangyum wangyum commented Jul 27, 2016

What changes were proposed in this pull request?

Spark will convert BooleanType to BIT(1), LongType to BIGINT, ByteType to BYTE when saving DataFrame to Oracle, but Oracle does not support BIT, BIGINT and BYTE types.

This PR is convert following Spark Types to Oracle types refer to Oracle Developer's Guide

Spark Type Oracle
BooleanType NUMBER(1)
IntegerType NUMBER(10)
LongType NUMBER(19)
FloatType NUMBER(19, 4)
DoubleType NUMBER(19, 4)
ByteType NUMBER(3)
ShortType NUMBER(5)

How was this patch tested?

Add new tests in JDBCSuite.scala and OracleDialect.scala

@srowen
Copy link
Member

srowen commented Jul 27, 2016

Jenkins test this please

@SparkQA
Copy link

SparkQA commented Jul 27, 2016

Test build #62919 has finished for PR 14377 at commit 22b0c2a.

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

@srowen
Copy link
Member

srowen commented Jul 29, 2016

@poolis @thomastechs do you have an opinion on this? I see you suggested changes to the Oracle dialect previously so might be good reviewers

// this to NUMERIC with -127 scale
// Not sure if there is a more robust way to identify the field as a float (or other
// numeric types that do not specify a scale.
case Types.NUMERIC if md.build().getLong("scale") == -127 =>
Copy link
Member

Choose a reason for hiding this comment

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

@wangyum I can merge this, since it looks reasonable, even though I'm not an Oracle expert. Can we streamline this a bit by only computing md.build().getLong("scale") once? The repeated matching of Types.NUMERIC could be done once in an if statement and be clearer.

@srowen
Copy link
Member

srowen commented Aug 3, 2016

Jenkins test this please

@SparkQA
Copy link

SparkQA commented Aug 3, 2016

Test build #63170 has finished for PR 14377 at commit 88e5d2e.

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

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Jenkins retest this please

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Jenkins add to whitelist

@SparkQA
Copy link

SparkQA commented Aug 5, 2016

Test build #63263 has finished for PR 14377 at commit bfadf77.

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

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Merged to master

@asfgit asfgit closed this in 39a2b2e Aug 5, 2016
@vivekdixit05
Copy link

This change has broken number with size to be converted to boolean. I have one table which has number column with size 1, in spark 2.1 its get converted to boolean, instead of integer. If this is expected, from the code is there a way to avoid this conversion?

@gatorsmile
Copy link
Member

@vivekdixit05 Which direction? Reading or writing?

@vivekdixit05
Copy link

vivekdixit05 commented May 31, 2017

@gatorsmile While reading.

@gatorsmile
Copy link
Member

@vivekdixit05 Does this PR #17830 resolve your issue?

case 1 => Option(BooleanType)
case 3 | 5 | 10 => Option(IntegerType)
case 19 if scale == 0L => Option(LongType)
case 19 if scale == 4L => Option(FloatType)
Copy link
Member

Choose a reason for hiding this comment

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

We should not change the read path. The bug we need to resolve is just the write path.

asfgit pushed a commit that referenced this pull request Jun 24, 2017
… in read path

## What changes were proposed in this pull request?

This PR is to revert some code changes in the read path of #14377. The original fix is #17830

When merging this PR, please give the credit to gaborfeher

## How was this patch tested?

Added a test case to OracleIntegrationSuite.scala

Author: Gabor Feher <[email protected]>
Author: gatorsmile <[email protected]>

Closes #18408 from gatorsmile/OracleType.

(cherry picked from commit b837bf9)
Signed-off-by: gatorsmile <[email protected]>
ghost pushed a commit to dbtsai/spark that referenced this pull request Jun 24, 2017
… in read path

## What changes were proposed in this pull request?

This PR is to revert some code changes in the read path of apache#14377. The original fix is apache#17830

When merging this PR, please give the credit to gaborfeher

## How was this patch tested?

Added a test case to OracleIntegrationSuite.scala

Author: Gabor Feher <[email protected]>
Author: gatorsmile <[email protected]>

Closes apache#18408 from gatorsmile/OracleType.
asfgit pushed a commit that referenced this pull request Jun 24, 2017
… in read path

This PR is to revert some code changes in the read path of #14377. The original fix is #17830

When merging this PR, please give the credit to gaborfeher

Added a test case to OracleIntegrationSuite.scala

Author: Gabor Feher <[email protected]>
Author: gatorsmile <[email protected]>

Closes #18408 from gatorsmile/OracleType.
robert3005 pushed a commit to palantir/spark that referenced this pull request Jun 29, 2017
… in read path

## What changes were proposed in this pull request?

This PR is to revert some code changes in the read path of apache#14377. The original fix is apache#17830

When merging this PR, please give the credit to gaborfeher

## How was this patch tested?

Added a test case to OracleIntegrationSuite.scala

Author: Gabor Feher <[email protected]>
Author: gatorsmile <[email protected]>

Closes apache#18408 from gatorsmile/OracleType.
@zwessels
Copy link

zwessels commented Aug 15, 2017

NUMBER(10) shouldn't automatically map to Integer. In our case, we have values stored like 4001826725 in that column and thus we get java.sql.SQLException:Numeric Overflow.

@wangyum
Copy link
Member Author

wangyum commented Aug 15, 2017

@gatorsmile We can consider merging this PR: #18266.

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.

6 participants