Fix NPE reading STRING COLLATE columns via Arrow

[msprunck]

Description

Fixes #1547.

A STRING column carrying a non-default collation (e.g. STRING COLLATE UTF8_LCASE) cannot be read on 3.4.1. Such a column is reported with a type_name that does not map to any ColumnInfoTypeName, so ColumnInfo.getTypeName() is null. Two things break:

1. Value read — ArrowToJavaObjectConverter.convert reaches switch (requiredType) with requiredType == null and throws a NullPointerException. This aborts every result set containing a collated string column, including a single-row result.
2. Metadata — DatabricksResultSetMetaData maps the null type name to Types.OTHER, so getColumnType() returns OTHER and the column is not usable as a string, even once the value read is fixed.

Why 3.4.1 surfaces this

3.4.1 routes metadata/type resolution through the SEA result manifest (the SQL SHOW / SEA metadata unification described in the 3.4.1 breaking changes) rather than native Thrift metadata RPCs. That path returns the column's server type text verbatim (STRING COLLATE UTF8_LCASE, reported over Arrow as type_name = UNKNOWN_COLUMN_TYPE_NAME), which the driver's type mapping does not recognize. Older drivers took a metadata path that yielded a mappable STRING type, so the same query/column worked. The driver's conversion source is otherwise unchanged between the two versions — the trigger is the metadata routing, not the conversion logic.

Changes

• ArrowToJavaObjectConverter.convert: when requiredType is null and the arrow metadata starts with STRING (covering STRING COLLATE ...), resolve the type to STRING, mirroring the existing prefix-based recovery for ARRAY/STRUCT/MAP/VARIANT/TIMESTAMP/GEOMETRY/GEOGRAPHY. Also guard the switch with an explicit DatabricksValidationException (logging only the metadata, not the raw cell value) so any future unmapped type produces a clear error instead of a raw NPE.
• DatabricksResultSetMetaData (SEA result manifest constructor): when the column type name is null and the type text starts with STRING, recover STRING so getColumnType() resolves to VARCHAR instead of OTHER. The collated type text is preserved so getColumnTypeName() still reports the server type (mirroring the existing TIMESTAMP_NTZ null-recovery).

Testing

• ArrowToJavaObjectConverterTest#testCollatedStringWithNullRequiredType — reads a collated column value with requiredType = null, asserts the string is returned rather than an NPE.
• ArrowToJavaObjectConverterTest#testUnresolvableTypeThrowsValidationException — asserts the null-guard throws DatabricksValidationException for a genuinely unmappable type.
• DatabricksResultSetMetaDataTest#testColumnsWithCollatedString — asserts getColumnType() is VARCHAR and getColumnTypeName() preserves STRING COLLATE UTF8_LCASE.
• ArrowToJavaObjectConverterTest and DatabricksResultSetMetaDataTest pass (64 tests).
• Validated end-to-end against a live SQL warehouse whose view exposes rule_name COLLATE UTF8_LCASE / rule_description COLLATE UTF8_LCASE: with this change on 3.4.1, value reads succeed and getColumnType() reports VARCHAR, where the unpatched 3.4.1 driver throws the NPE.

Additional Notes to the Reviewer

• Root cause was bisected to the 3.4.1 metadata routing change (not the SDK 0.69→0.106 bump, which was verified not to affect this). The regression reproduces purely on the advertised driver version / metadata path; the converter bytecode is identical across the versions.
• The STRING-prefix branches are guarded by a null type check, so they act only as a fallback and do not change behavior for columns whose type already resolves.
• Keying off the type text / arrow metadata prefix (rather than adding a new enum value) matches the existing convention in both methods.

[vikrantpuppala]

Medium · flagged by devil's-advocate + test reviewers, confirmed against head SHA.

The two recovery sites added in this PR normalize case differently:

• Value path (this line, ArrowToJavaObjectConverter.java:96): arrowMetadata.startsWith(STRING) — case-sensitive.
• Metadata path (DatabricksResultSetMetaData.java:113): columnInfo.getTypeText().toUpperCase().startsWith(STRING) — case-insensitive.

If any server build emits the collated type text in lower/mixed case (e.g. "string collate utf8_lcase"), the metadata path resolves the column to VARCHAR, but this value path's startsWith fails, requiredType stays null, and getObject throws DatabricksValidationException. The driver would then advertise a readable VARCHAR column that cannot actually be read — worse than a clean failure.

Suggestion: align both sites on the same case handling. Since the value-path siblings (ARRAY/STRUCT/MAP/…) are all case-sensitive by the Spark SqlName uppercase convention, either accept that in both places or normalize both with toUpperCase(Locale.ROOT).

---

_{Posted by code-review-squad · /full-review · feedback: #code-review-squad-feedback}

[vikrantpuppala]

Medium (open question) · flagged by devil's-advocate; consequence not verified at the branching point.

The read path (ArrowStreamResult.java:251-253) falls back to columnInfo.getTypeText() only when chunkIterator.getType() returns null — not when it returns a non-null value that maps to no known type. So this recovery keys off arrowMetadata (the Arrow field Spark:DataType:SqlName), which may differ from the manifest type_text.

Issue #1547 notes two warehouse builds on the same version label behaved differently (one NPE'd, one didn't). If some build's Spark:DataType:SqlName for a collated column is a value that doesn't start with "STRING", this recovery won't fire and the read now throws DatabricksValidationException instead of the old NPE — still unreadable for the user.

I could not confirm what Spark:DataType:SqlName actually holds for the failing build (UNKNOWN_COLUMN_TYPE_NAME is a protobuf/SDK enum sentinel → null after deserialization, not an Arrow-metadata string), so this is a question rather than a confirmed defect.

Question: for the exact warehouse build in #1547 that NPE'd, what does the Arrow field metadata Spark:DataType:SqlName contain for the collated column? If it can be non-STRING, consider recovering when either arrowMetadata or columnInfo.getTypeText() starts with "STRING".

---

_{Posted by code-review-squad · /full-review · feedback: #code-review-squad-feedback}

[vikrantpuppala]

Medium · flagged by architecture + maintainability reviewers.

This recovery now lives in both DatabricksResultSetMetaData and ArrowToJavaObjectConverter because the SEA path has no single type-name resolution point — the metadata constructor recovers into a local variable and never writes back to the shared ColumnInfo, so the read path independently re-derives the null getTypeName(). Every future null-type_name server type must be patched in lockstep, and the two copies have already drifted on case handling (see F1).

This mirrors a pre-existing convention, so it's a maintainability cost to note rather than a blocker. Suggested direction (follow-up, not required here): a single resolveTypeName(ColumnInfo) helper applied right after SEA manifest deserialization that calls setTypeName(...) on the shared object, letting both consumers drop their local recovery.

---

_{Posted by code-review-squad · /full-review · feedback: #code-review-squad-feedback}

[vikrantpuppala]

Low · flagged by devil's-advocate.

A hypothetical future type whose name begins with the letters STRING (e.g. "STRINGVIEW", "STRINGSET") would be silently coerced to scalar STRING and read via toString() — producing wrong values rather than an error, precisely on the null-typeName path where a brand-new type is most likely to land. Consistent with the existing sibling prefix checks, so low priority.

Optional: match a word boundary — typeText.equals("STRING") || typeText.startsWith("STRING ") || typeText.startsWith("STRING(") — so "STRING COLLATE …" matches but "STRINGVIEW" does not.

---

_{Posted by code-review-squad · /full-review · feedback: #code-review-squad-feedback}

[vikrantpuppala]

Low · flagged by test reviewer.

Both new tests feed already-uppercase literals, so:

• The .toUpperCase() on the metadata side is a no-op in the test — delete it from production and the test still passes (covered by zero assertions).
• Plain "STRING" via the null-recovery path is uncovered (the existing testNullHandlingInVarCharVector uses a non-null requiredType).

A converter case with lowercase metadata — convert(vector, 0, null, "string collate utf8_lcase", new ColumnInfo()) expecting the value — would currently fail and directly surface F1. Adding a lowercase metadata assertion covers the .toUpperCase() call. The two added tests are otherwise sound (real Arrow vectors, independent literal expectations, would fail if the fix were reverted).

---

_{Posted by code-review-squad · /full-review · feedback: #code-review-squad-feedback}

[vikrantpuppala]

Code Review Squad — Review

Score: 89/100 — MODERATE RISK

Solid, well-scoped bug fix that faithfully follows the existing in-file prefix-recovery convention. No merge blockers — every finding below is a robustness or test-coverage gap around the recovery logic, verified against the PR head SHA. The highest-value item is the case-asymmetry between the two recovery sites (F1); the lowercase test in F5 would have caught it for free. Security and language reviewers found nothing at threshold.

---