Skip to content

SQL: Support for coercing to DECIMAL.#4028

Merged
jon-wei merged 1 commit intoapache:masterfrom
gianm:sql-decimal
Mar 13, 2017
Merged

SQL: Support for coercing to DECIMAL.#4028
jon-wei merged 1 commit intoapache:masterfrom
gianm:sql-decimal

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Mar 8, 2017

Useful for running queries that involve math of ints and floats, which
Calcite types as decimal. (see the test for an example)

Useful for running queries that involve math of ints and floats, which
Calcite types as decimal.
@gianm gianm added this to the 0.10.1 milestone Mar 8, 2017
@jihoonson
Copy link
Copy Markdown
Contributor

SQL's DECIMAL type requires precision and scale, but it seems to be simply translated into Java's double type in this patch. Would you share your plan for supporting decimal type?

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Mar 9, 2017

I have no plan for that, this is coercion rather than real standard compliant support. I think it'd require changes to Druid core, not just the SQL layer, since it's not a type supported by Druid.

@jihoonson
Copy link
Copy Markdown
Contributor

jihoonson commented Mar 9, 2017

Right. The simplest solution would be adding a BigDecimal type to Druid and use it for SQL's decimal type. We can do or devise more efficient way later.

The patch looks good to me.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 10, 2017

👍

1 similar comment
@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Mar 13, 2017

👍

@jon-wei jon-wei merged commit bad250f into apache:master Mar 13, 2017
gianm added a commit to implydata/druid-public that referenced this pull request Apr 13, 2017
Useful for running queries that involve math of ints and floats, which
Calcite types as decimal.
@gianm gianm deleted the sql-decimal branch September 23, 2022 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants