Skip to content

Conversation

@itholic
Copy link
Contributor

@itholic itholic commented Feb 3, 2023

What changes were proposed in this pull request?

This PR proposes to integrate _LEGACY_ERROR_TEMP_1229 into DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION.

_LEGACY_ERROR_TEMP_1229

  "_LEGACY_ERROR_TEMP_1229" : {
    "message" : [
      "<decimalType> can only support precision up to <precision>."
    ]
  },

DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION

  "DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION" : {
    "message" : [
      "Decimal precision <precision> exceeds max precision <maxPrecision>."
    ],
    "sqlState" : "22003"
  },

Why are the changes needed?

We should assign proper name to LEGACY_ERROR_TEMP*

Does this PR introduce any user-facing change?

No

How was this patch tested?

./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*"

@itholic
Copy link
Contributor Author

itholic commented Feb 3, 2023

I think we can reuse DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION for _LEGACY_ERROR_TEMP_1229.

cc @srielau @MaxGekk @cloud-fan

Copy link
Contributor

@srielau srielau left a comment

Choose a reason for hiding this comment

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

LGTM

val e = intercept[SparkArithmeticException](defaultParser.parseExpression("1.20E-38BD"))
checkError(
exception = parseException("1.20E-38BD"),
errorClass = "_LEGACY_ERROR_TEMP_0061",
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I didn't get. The PR title and description are telling only about one error class LEGACY_ERROR_TEMP_1229 but your PR affects another one. Could you elaborate this and revert unrelated changes, or make PR's title/description consistent with PR's changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right.
I should have fixed sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala rather than tests.
Just reverted unrelated changes and fix them in proper way.
Thanks!

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM

@MaxGekk
Copy link
Member

MaxGekk commented Feb 8, 2023

+1, LGTM. Merging to master/3.4.
Thank you, @itholic and @srielau @LuciferYang for review.

@MaxGekk MaxGekk closed this in f8e06c1 Feb 8, 2023
MaxGekk pushed a commit that referenced this pull request Feb 8, 2023
…PRECISION_EXCEEDS_MAX_PRECISION`

### What changes were proposed in this pull request?

This PR proposes to integrate `_LEGACY_ERROR_TEMP_1229` into `DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION`.

**_LEGACY_ERROR_TEMP_1229**
```json
  "_LEGACY_ERROR_TEMP_1229" : {
    "message" : [
      "<decimalType> can only support precision up to <precision>."
    ]
  },
```

**DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION**
```json
  "DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION" : {
    "message" : [
      "Decimal precision <precision> exceeds max precision <maxPrecision>."
    ],
    "sqlState" : "22003"
  },
```

### Why are the changes needed?

We should assign proper name to _LEGACY_ERROR_TEMP_*

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

`./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*"`

Closes #39875 from itholic/LEGACY_1229.

Authored-by: itholic <haejoon.lee@databricks.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit f8e06c1)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@itholic itholic deleted the LEGACY_1229 branch April 22, 2023 05:47
snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
…PRECISION_EXCEEDS_MAX_PRECISION`

### What changes were proposed in this pull request?

This PR proposes to integrate `_LEGACY_ERROR_TEMP_1229` into `DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION`.

**_LEGACY_ERROR_TEMP_1229**
```json
  "_LEGACY_ERROR_TEMP_1229" : {
    "message" : [
      "<decimalType> can only support precision up to <precision>."
    ]
  },
```

**DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION**
```json
  "DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION" : {
    "message" : [
      "Decimal precision <precision> exceeds max precision <maxPrecision>."
    ],
    "sqlState" : "22003"
  },
```

### Why are the changes needed?

We should assign proper name to _LEGACY_ERROR_TEMP_*

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

`./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*"`

Closes apache#39875 from itholic/LEGACY_1229.

Authored-by: itholic <haejoon.lee@databricks.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit f8e06c1)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
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