[SPARK-46519][SQL] Clear unused error classes from `error-classes.json` file #44503

panbingkun · 2023-12-27T03:05:29Z

What changes were proposed in this pull request?

The pr aims to:

Clear unused error classes from error-classes.json.
Delete unused methods dataSourceAlreadyExists in QueryCompilationErrors.scala
Fix an outdated comment.

Why are the changes needed?

Make code clear.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.
Manually test.

Was this patch authored or co-authored using generative AI tooling?

No.

… file

panbingkun · 2023-12-27T03:28:40Z

How to find unused error classes in Spark code repo
1.Generate the names of all error classes based on error classes. json.
2.Use a shell script to locate each of the above error classes in the Spark code repo and identify any unused error classes.
3.Manually check again.

During the search process above, some unused error classes were also discovered, as follows:

INTERNAL_ERROR_BROADCAST
INTERNAL_ERROR_MEMORY
INTERNAL_ERROR_STORAGE
INTERNAL_ERROR_SHUFFLE
INTERNAL_ERROR_EXECUTOR

These error classes did not appear in the Spark code repo, but they are temporarily retained as they are internal errors.

panbingkun · 2023-12-27T03:30:52Z

sql/api/src/main/scala/org/apache/spark/sql/errors/DataTypeErrors.scala

 /**
 * Object for grouping error messages from (most) exceptions thrown during query execution.
 * This does not include exceptions thrown during the eager execution of commands, which are
- * grouped into [[QueryCompilationErrors]].


In the sql/api module, QueryCompilationErrors has been renamed to CompilationErrors`, so we will fix it here.

We have both, AFAIK

From the perspective of module dependency, it seems to express the meaning of CompilationErrors in the sql/api module, instead of QueryCompilationErrors in the sql/catalyst module.

Or we can write it as: [[CompilationErrors]] and [[QueryCompilationErrors]] ?

The most primitive PR is here: https://github.com/apache/spark/pull/41794/files#diff-1b8affb347590e3ccabe359b07215c72aead8108ca3a5a020b3cb71e639c117bR25

ok, let's leave the ref to CompilationErrors only.

panbingkun · 2023-12-27T03:31:18Z

cc @MaxGekk

… file

panbingkun · 2023-12-27T06:35:15Z

@MaxGekk
Additionally, I have found two error classes that only appear in UT. In my opinion, both of these error classes should be deleted. Is this appropriate？

1._LEGACY_ERROR_TEMP_3066

spark/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala

Line 572 in 5a5d3c7

errorClass = "_LEGACY_ERROR_TEMP_3066",

2._LEGACY_ERROR_TEMP_3078

spark/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala

Line 163 in 5a5d3c7

    
           (_: LogicalPlan) => throw new AnalysisException("_LEGACY_ERROR_TEMP_3078", Map.empty))

spark/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilterSuite.scala

Line 2349 in 5a5d3c7

errorClass = "_LEGACY_ERROR_TEMP_3078", messageParameters = Map.empty)

MaxGekk

These error classes did not appear in the Spark code repo, but they are temporarily retained as they are internal errors.

FYI, we have a conception of category for internal error, see https://github.com/apache/spark/pull/40978/files#diff-41229d3f8af21d2eb979e6d8c5b52058b1c460508f1786a6efa8dd6dbbc8c218R79. We form the names dynamically based on the caller side. This should allow to quickly identify from which sub-system the error comes from and who is responsible for it (to handle it).

MaxGekk · 2023-12-27T06:43:50Z

both of these error classes should be deleted

If it is possible to delete them while preserving test logic, let's delete them.

… file

panbingkun · 2023-12-27T10:47:31Z

...e/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilterSuite.scala

-        case _ =>
-          throw new AnalysisException(
-            errorClass = "_LEGACY_ERROR_TEMP_3078", messageParameters = Map.empty)
+        case _ => assert(false, "Can not match ParquetTable in the query.")


Refer to this:

spark/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcFilterSuite.scala

Line 73 in b106f80

case _ => assert(false, "Can not match OrcTable in the query.")

panbingkun · 2023-12-27T10:51:09Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala

          throw new AnalysisException(
-            errorClass = "_LEGACY_ERROR_TEMP_3066",
-            messageParameters = Map("msg" -> ex.getMessage))
+            errorClass = "_LEGACY_ERROR_TEMP_2193",


Refer to this:

spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala

Lines 383 to 384 in b106f80

case ex: MetaException =>

throw QueryExecutionErrors.getPartitionMetadataByFilterError(ex)

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala

Lines 1644 to 1651 in b106f80

def getPartitionMetadataByFilterError(e: Exception): SparkRuntimeException = {

new SparkRuntimeException(

errorClass = "_LEGACY_ERROR_TEMP_2193",

messageParameters = Map(

"hiveMetastorePartitionPruningFallbackOnException" ->

SQLConf.HIVE_METASTORE_PARTITION_PRUNING_FALLBACK_ON_EXCEPTION.key),

cause = e)

}

panbingkun · 2023-12-27T10:53:23Z

sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala

    // Throw an AnalysisException - this should be captured.
    spark.experimental.extraStrategies = Seq[SparkStrategy](
-      (_: LogicalPlan) => throw new AnalysisException("_LEGACY_ERROR_TEMP_3078", Map.empty))
+      (_: LogicalPlan) => throw new AnalysisException(


The UT can actually handle any type of AnalysisException. I have chosen an error class (UNSUPPORTED_DATASOURCE_FOR_DIRECT_QUERY) that is closer to semantics to replace it.

MaxGekk · 2023-12-27T12:24:10Z

common/utils/src/main/resources/error/error-classes.json

    },
    "sqlState" : "42K0B"
  },
-  "INCORRECT_END_OFFSET" : {


For some reasons there is still a reference to the error class:

$ find . -type f -print0|xargs -0 grep INCORRECT_END_OFFSET ./docs/sql-error-conditions-sqlstates.md: <td><a href="arithmetic-overflow-error-class.md">ARITHMETIC_OVERFLOW</a>, <a href="sql-error-conditions.html#cast_overflow">CAST_OVERFLOW</a>, <a href="sql-error-conditions.html#cast_overflow_in_table_insert">CAST_OVERFLOW_IN_TABLE_INSERT</a>, <a href="sql-error-conditions.html#decimal_precision_exceeds_max_precision">DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION</a>, <a href="sql-error-conditions.html#invalid_index_of_zero">INVALID_INDEX_OF_ZERO</a>, <a href="sql-error-conditions.html#incorrect_end_offset">INCORRECT_END_OFFSET</a>, <a href="sql-error-conditions.html#incorrect_ramp_up_rate">INCORRECT_RAMP_UP_RATE</a>, <a href="invalid-array-index-error-class.md">INVALID_ARRAY_INDEX</a>, <a href="invalid-array-index-in-element-at-error-class.md">INVALID_ARRAY_INDEX_IN_ELEMENT_AT</a>, <a href="sql-error-conditions.html#numeric_out_of_supported_range">NUMERIC_OUT_OF_SUPPORTED_RANGE</a>, <a href="sql-error-conditions.html#numeric_value_out_of_range">NUMERIC_VALUE_OUT_OF_RANGE</a>

Please, remove the reference to the deleted error class.

Okay, let me delete it first, and then I'll try to investigate the root cause again to see if we can automate its discovery.

MaxGekk · 2023-12-27T12:36:31Z

sql/api/src/main/scala/org/apache/spark/sql/errors/DataTypeErrors.scala

 /**
 * Object for grouping error messages from (most) exceptions thrown during query execution.
 * This does not include exceptions thrown during the eager execution of commands, which are
- * grouped into [[QueryCompilationErrors]].


ok, let's leave the ref to CompilationErrors only.

MaxGekk · 2023-12-27T17:10:08Z

common/utils/src/main/resources/error/error-classes.json

    },
    "sqlState" : "42K0B"
  },
-  "INCORRECT_END_OFFSET" : {


Please, remove the reference to the deleted error class.

… file

MaxGekk · 2023-12-28T08:12:12Z

+1, LGTM. Merging to master.
Thank you, @panbingkun.

[SPARK-46519][SQL] Clear unused error classes from error-classes.json…

286dfdc

… file

github-actions bot added SQL DOCS labels Dec 27, 2023

panbingkun commented Dec 27, 2023

View reviewed changes

panbingkun marked this pull request as ready for review December 27, 2023 03:31

panbingkun marked this pull request as draft December 27, 2023 05:40

[SPARK-46519][SQL] Clear unused error classes from error-classes.json…

9f5bab1

… file

MaxGekk reviewed Dec 27, 2023

View reviewed changes

[SPARK-46519][SQL] Clear unused error classes from error-classes.json…

557713d

… file

panbingkun commented Dec 27, 2023

View reviewed changes

panbingkun marked this pull request as ready for review December 27, 2023 10:53

panbingkun requested a review from MaxGekk December 27, 2023 10:53

MaxGekk reviewed Dec 27, 2023

View reviewed changes

MaxGekk requested changes Dec 27, 2023

View reviewed changes

panbingkun added 2 commits December 28, 2023 09:17

[SPARK-46519][SQL] Clear unused error classes from error-classes.json…

72e223f

… file

Merge branch 'master' into SPARK-46519

3578d3d

panbingkun requested a review from MaxGekk December 28, 2023 01:20

MaxGekk approved these changes Dec 28, 2023

View reviewed changes

MaxGekk closed this in bb497eb Dec 28, 2023

	case ex: MetaException =>
	throw QueryExecutionErrors.getPartitionMetadataByFilterError(ex)

	def getPartitionMetadataByFilterError(e: Exception): SparkRuntimeException = {
	new SparkRuntimeException(
	errorClass = "_LEGACY_ERROR_TEMP_2193",
	messageParameters = Map(
	"hiveMetastorePartitionPruningFallbackOnException" ->
	SQLConf.HIVE_METASTORE_PARTITION_PRUNING_FALLBACK_ON_EXCEPTION.key),
	cause = e)
	}

[SPARK-46519][SQL] Clear unused error classes from error-classes.json file #44503

[SPARK-46519][SQL] Clear unused error classes from error-classes.json file #44503

Uh oh!

Conversation

panbingkun commented Dec 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

panbingkun commented Dec 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panbingkun commented Dec 27, 2023

Uh oh!

panbingkun commented Dec 27, 2023

Uh oh!

MaxGekk left a comment

Choose a reason for hiding this comment

Uh oh!

MaxGekk commented Dec 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxGekk commented Dec 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[SPARK-46519][SQL] Clear unused error classes from `error-classes.json` file #44503

[SPARK-46519][SQL] Clear unused error classes from `error-classes.json` file #44503

panbingkun commented Dec 27, 2023 •

edited

Loading