Skip to content

Conversation

@wangtao605
Copy link

@wangtao605 wangtao605 commented Jul 14, 2018

What changes were proposed in this pull request?

numerical is as same with decimal. Spark has already supported decimal,so i think we should add support for numeric to align SQL standards.

(Please fill in changes proposed in this fix)

How was this patch tested?

already exist tests

Please review http://spark.apache.org/contributing.html before opening a pull request.

@wangtao605 wangtao605 changed the title [SPARK-24803] add support for numeric [SPARK-24803][SQL] add support for numeric Jul 14, 2018
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@maropu
Copy link
Member

maropu commented Jul 14, 2018

@gatorsmile Is it worth adding a numeric type as an alias of decimal? Both types seems to be in the SQL standard and some dbms (e.g., postgresql and sql server) can parse both.
@wangtao605 Can you add tests to check if these types can be parsed correctly?

@gatorsmile
Copy link
Member

gatorsmile commented Jul 14, 2018

Although MS SQL Server looks like use them interchangeably, https://docs.microsoft.com/en-us/sql/t-sql/data-types/decimal-and-numeric-transact-sql?view=sql-server-2017, they are not exactly the same in ANSI SQL.

  1. NUMERIC species the data type exact numeric, with the decimal precision and scale specified by the and .
  2. DECIMAL species the data type exact numeric, with the decimal scale specified by the and the implementation-de ned decimal precision equal to or greater than the value of the specified .

@maropu
Copy link
Member

maropu commented Jul 15, 2018

aha, I didn't know that and postgresql also uses them interchangeably, too.

@dmateusp
Copy link

Just checked out the PR,

scala> spark.sql("SELECT CAST(1 as NUMERIC)")
res0: org.apache.spark.sql.DataFrame = [CAST(1 AS DECIMAL(10,0)): decimal(10,0)]

scala> spark.sql("SELECT NUMERIC(1)")
org.apache.spark.sql.AnalysisException: Undefined function: 'NUMERIC'. This function is neither a registered temporary function nor a permanent function registered in the database 'default'.; line 1 pos 7

I imagine some tests could be added here:

  • sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DataTypeParserSuite.scala
  • sql/core/src/test/resources/sql-tests/inputs/

Do you think it's worth having a separate DataType or just have it as an alias?

@rxin
Copy link
Contributor

rxin commented Jul 16, 2018

Why did you need this change? Given it's very difficult to revert the change (or introduce a proper numeric type if ever needed in the future), I would not merge this pull request unless there are sufficient justification.

@wangtao605
Copy link
Author

@dmateusp Actually i think "Numeric" has no essential difference with "Decimal". May be just have it as an alias is better,i will add some tests if you agree.

@wangtao605
Copy link
Author

@rxin In order to support sql syntax better and align SQL standards. I think it is worth to add a numeric type as an alias of decimal.

@asfgit asfgit closed this in a3ba3a8 Nov 11, 2018
@gatorsmile
Copy link
Member

@wangtao605 Do you mind documenting our behavior in our Spark SQL doc?

@wangtao605
Copy link
Author

@wangtao605 Do you mind documenting our behavior in our Spark SQL doc?

Yes, it's ok.

zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
Closes apache#21766
Closes apache#21679
Closes apache#21161
Closes apache#20846
Closes apache#19434
Closes apache#18080
Closes apache#17648
Closes apache#17169

Add:
Closes apache#22813
Closes apache#21994
Closes apache#22005
Closes apache#22463

Add:
Closes apache#15899

Add:
Closes apache#22539
Closes apache#21868
Closes apache#21514
Closes apache#21402
Closes apache#21322
Closes apache#21257
Closes apache#20163
Closes apache#19691
Closes apache#18697
Closes apache#18636
Closes apache#17176

Closes apache#23001 from wangyum/CloseStalePRs.

Authored-by: Yuming Wang <yumwang@ebay.com>
Signed-off-by: hyukjinkwon <gurwls223@apache.org>
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