[SPARK-47300][SQL] quoteIfNeeded should quote identifier starts with digits

[cloud-fan]

What changes were proposed in this pull request?

quoteIfNeeded is used to generate pretty strings of identifiers in error message, EXPLAIN result, etc. It's mostly for humans to read but sometimes people may copy-paste the identifier string and put it in a SQL. There is a small issue in quoteIfNeeded: it does not quote number literals like 0d, and people will get parse exception if they directly put the identifier string in SQL.

This PR fixes the issue by always quoting the identifier if it does not start with alphabet or underscore. Note, there might be program trying to parse the identifier string, but this change is safe as these programs should already handle quoting and it's ok to quote more.

Why are the changes needed?

make identifier string parsable is more user-friendly.

Does this PR introduce any user-facing change?

yes, the identifier string in the error message and EXPLAIN result will be properly quoted.

How was this patch tested?

new test suite

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

no

[cloud-fan]

cc @yaooqinn @dongjoon-hyun

[yaooqinn]

spark-sql (default)> select version();
3.5.0 ce5ddad990373636e94071e7cef2f31021add07b
spark-sql (default)> create table d (0a real) using parquet;
spark-sql (default)> desc formatted d;
0a                  	float

Do we already have it?

[cloud-fan]

@yaooqinn 0a is fine but 0d is not, as it's double literal.

[yaooqinn]

LGTM

[MaxGekk]

@cloud-fan Seems the test failure is related:

[info] - SPARK-34872: quoteIfNeeded should quote a string which contains non-word characters *** FAILED *** (3 milliseconds)
[info]   "[`1a`]" did not equal "[1a]" (StringUtilsSuite.scala:136)
[info]   Analysis:
[info]   "[`1a`]" -> "[1a]"

[MaxGekk]

How about deduplicate the tests:

spark/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/StringUtilsSuite.scala

Lines 132 to 140 in 5089140

	test("SPARK-34872: quoteIfNeeded should quote a string which contains non-word characters") {
	assert(quoteIfNeeded("a b") === "`a b`")
	assert(quoteIfNeeded("a*b") === "`a*b`")
	assert(quoteIfNeeded("123") === "`123`")
	assert(quoteIfNeeded("1a") === "1a")
	assert(quoteIfNeeded("_ab_") === "_ab_")
	assert(quoteIfNeeded("_") === "_")
	assert(quoteIfNeeded("") === "``")
	}

and maybe move them to QuotingUtilsSuite.

[dongjoon-hyun]

It seems that the same failure (which @MaxGekk 's reported) still exists at the last CI.

StringUtilsSuite:
[info] - escapeLikeRegex (1 millisecond)
[info] - filter pattern (1 millisecond)
[info] - string concatenation (1 millisecond)
[info] - string concatenation with limit (1 millisecond)
[info] - string concatenation return value (1 millisecond)
[info] - SPARK-31916: StringConcat doesn't overflow on many inputs (190 milliseconds)
[info] - SPARK-31916: verify that PlanStringConcat's output shows the actual length of the plan (3 milliseconds)
[info] - SPARK-34872: quoteIfNeeded should quote a string which contains non-word characters *** FAILED *** (2 milliseconds)
[info]   "[`1a`]" did not equal "[1a]" (StringUtilsSuite.scala:136)
[info]   Analysis:
[info]   "[`1a`]" -> "[1a]"

[dongjoon-hyun]

+1, LGTM (if CIs passes after adjusting the test suite)

[cloud-fan]

it's covered by the new test

[yaooqinn]

Merged to master.

Thank you @cloud-fan @dongjoon-hyun @HyukjinKwon @MaxGekk