spark-test commented on code in PR #60059: URL: https://github.com/apache/spark/pull/60059#discussion_r4488432328
########## docs/sql-migration-guide.md: ########## @@ -52,6 +52,7 @@ license: | - Since Spark 4.4, the Oracle JDBC datasource maps bare `NUMBER` columns (declared without explicit precision or scale) to `DecimalType(38, 18)` instead of `DecimalType(38, 10)`. This uses `DecimalType.DEFAULT_SCALE` and preserves up to 18 fractional digits instead of 10, reducing silent precision loss. However, the integer range shrinks from 28 to 20 digits; bare `NUMBER` values with more than 20 integer digits that previously read correctly will raise `NUMERIC_VALUE_OUT_OF_RANGE`. The same change applies to Oracle `FLOAT` columns, which JDBC reports as `NUMERIC` with `scale = -127`. Columns with explicit `NUMBER(p,s)` are not affected. To restore the previous `DecimalType(38, 10)` mapping, set `spark.sql.legacy.oracle.numberMapping.enabled` to `true`. - Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is false and `spark.sql.readSideCharPadding` is true (the default), JDBC Data Source V2 catalog tables on H2, PostgreSQL, SQL Server, and Oracle push CHAR equality, inequality, null-safe equality, and `IN`/`NOT IN`. In 4.3 the default read-side padding wrap kept those filters in Spark. Pushed predicates use the remote collation, so a case-insensitive SQL Server collation can match a stored `'ABC'` with `flag = 'abc'` while a Spark-local comparison would not. The V1 path (`spark.read.jdbc` and `USING jdbc`) still evaluates these CHAR filters in Spark. PostgreSQL's internal `"char"` type (JDBC type name `char`) is mapped to `STRING` instead of `CHAR(1)`, so Spark no longer blank-pads its empty value. - Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is false and `spark.sql.readSideCharPadding` is true (the default), `IS [NOT] NULL` over a CHAR scan column is unwrapped to the raw column for every Data Source V2 source, including every JDBC dialect. Previously the padding wrap kept that null check in Spark. +- Since Spark 4.4, a CHAR/VARCHAR column of the outer query that a LATERAL or correlated subquery returns keeps its CHAR/VARCHAR type, as an inner column does. Previously it became `STRING`: comparisons against it were not padded and could silently return no rows, and CTAS and views over it stored `STRING`. Inside a LATERAL subquery this affected `t.c AS k`, `t.c` and `t.*` in all earlier versions; in other correlated subqueries it affected `t.c AS k` in all earlier versions, `t.*` since Spark 4.2 and `t.c` since Spark 4.3. A table created with CTAS over such a column now gets `CHAR(n)`/`VARCHAR(n)`, so later inserts are length-checked (`EXCEED_LIMIT_LENGTH`) and CHAR values are padded. To keep `STRING`, cast the column in the query, for example `CAST(t.c AS STRING) AS k`. Permanent views created by an earlier version keep the `STRING` type they stored, so comparisons against a CHAR column of such a view still return no rows until the view is recreated with `CREATE OR REPLACE VIEW` or `ALTER VIEW ... AS`. Review Comment: **Nit (P3):** The last sentence holds only for views outside `WITH SCHEMA EVOLUTION`. For a view created `WITH SCHEMA EVOLUTION`, `SessionCatalog.fromCatalogTable` returns the re-parsed view query without the `castColToType` projection that re-applies the stored column metadata. The view's output therefore comes straight from its re-analyzed query, which now carries the forwarded CHAR/VARCHAR key. `ViewSyncSchemaToMetaStore` ignores metadata-only differences, so only the stored schema, and what `DESCRIBE` shows, stays STRING. For example, take a view created by an earlier version as `CREATE VIEW v WITH SCHEMA EVOLUTION AS SELECT l.k FROM t, LATERAL (SELECT t.c AS k) l` over `c CHAR(5)` holding `'ab'`. After the upgrade, `SELECT * FROM v WHERE k = 'ab'` returns the row, and a CTAS over `v` stores `CHAR(5)` with length-checked inserts. The note tells users of such views that nothing changes until they recreate them, so they may misattribute the change, or skip the `CAST` they need to keep STRING downstream. The sentence could exclude these views and say that they pick up the CHAR/VARCHAR type on their first read after the upgrade. This extends existing discussions [1](https://github.com/apache/spark/pull/60059#issuecomment-6664826707), [2](https://github.com/apache/spark/pull/60059#pullrequestreview-5779475438). **Verification:** - **Inspection:** The migration-guide entry separates views created WITH SCHEMA EVOLUTION, which pick up the CHAR/VARCHAR type on their first read after the upgrade, from views in the other schema modes, which keep the stored STRING type, and so unpadded CHAR comparisons, until they are recreated. ########## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala: ########## @@ -463,6 +463,13 @@ case class OuterReference(e: NamedExpression) extends LeafExpression with NamedExpression with Unevaluable { override def dataType: DataType = e.dataType override def nullable: Boolean = e.nullable + // Forward only the raw CHAR/VARCHAR type when aliased. Other keys are not forwarded; some, + // such as the SQL UDF parameter and auto-generated alias markers, must not leak. + override def metadata: Metadata = CharVarcharUtils.getRawTypeString(e.metadata) match { Review Comment: Thanks, the migration note now names the exemption and the recreate step. The other half is still open: castColToType, where the stored view metadata wins, has no TODO naming the stored-schema migration these older views need. Without it, nothing on the view read path records that the fix does not reach them yet. <!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4471060884","thread_id":"inline:4471060884","verdict_sha256":"275257a8f06f569c9503015aaa202d34af4d6e71290ecf32ee6a342ae54b9bf7"} --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
