Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Jul 24, 2015

Right now, we use double to parse all the float number in SQL. When it's used in expression together with DecimalType, it will turn the decimal into double as well. Also it will loss some precision when using double.

This PR change to parse float number to decimal or double, based on it's using scientific notation or not, see https://msdn.microsoft.com/en-us/library/ms179899.aspx

This is a break change, should we doc it somewhere?

@SparkQA
Copy link

SparkQA commented Jul 24, 2015

Test build #38337 has finished for PR 7642 at commit 1c48e34.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

typo 'e?

@SparkQA
Copy link

SparkQA commented Jul 24, 2015

Test build #1197 has finished for PR 7642 at commit f4a320b.

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

@SparkQA
Copy link

SparkQA commented Jul 24, 2015

Test build #38379 has finished for PR 7642 at commit eca99de.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class ChangeDecimalPrecision(child: Expression) extends UnaryExpression

@SparkQA
Copy link

SparkQA commented Jul 24, 2015

Test build #38384 has finished for PR 7642 at commit 5e142b6.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class ChangeDecimalPrecision(child: Expression) extends UnaryExpression

Conflicts:
	sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
@SparkQA
Copy link

SparkQA commented Jul 28, 2015

Test build #38679 has finished for PR 7642 at commit 1f576d9.

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

@davies
Copy link
Contributor Author

davies commented Jul 28, 2015

cc @yhuai @rxin

@rxin
Copy link
Contributor

rxin commented Jul 28, 2015

We should add "releasenotes" label to the JIRA ticket.

@rxin
Copy link
Contributor

rxin commented Jul 28, 2015

cc @cloud-fan for review too

Copy link
Contributor

Choose a reason for hiding this comment

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

should we apply this to CreateArray too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For CreateArray, it's good to make it strict. We can loose the rule later.

@cloud-fan
Copy link
Contributor

LGTM

@rxin
Copy link
Contributor

rxin commented Jul 29, 2015

@davies do we need to change the hive ql part?

@davies
Copy link
Contributor Author

davies commented Jul 29, 2015

@rxin HiveQl already does this, it always parse float number as decimal.

@rxin
Copy link
Contributor

rxin commented Jul 29, 2015

Great - merging this in. Thanks.

@asfgit asfgit closed this in 15667a0 Jul 29, 2015
@scwf
Copy link
Contributor

scwf commented Oct 14, 2015

hi @davies seems this is not compatible with hiveql, HiveQl still parse float number as double. https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveQl.scala#L1670-L1689

Or i missed sth?

@davies
Copy link
Contributor Author

davies commented Oct 14, 2015

Yes, If you use HiveContext, you will be compatible with Hive, but SQLContext will not.

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.

5 participants