srielau commented on PR #55866:
URL: https://github.com/apache/spark/pull/55866#issuecomment-4452791878
Thanks for the careful review @cloud-fan! All four items addressed in
542eee8:
- **`SqlPathV2CatalogSuite` first-match function test** — your read was
right, the two `StrLen` impls returned identical lengths so swapping path order
proved nothing. Now uses a tiny `StrLenTimes100` `ScalarFunction[Int]` on
`pathcat2` so `strlen('abc')` returns 3 or 300 depending on path order.
Structurally symmetric with the table test (id=10 vs id=20).
- **`SQLViewSuite` test names** — `SPARK-56853:` prefix restored on both new
tests; matches the file-local convention (your SPARK-56639 frozen-path tests
right next to them are feature tests with the prefix too, so the convention
isn't bug-regression-only).
- **`CatalogManagerSuite` / `PathElementSuite`** — taking the "update the PR
description" option. `PathElement` is a small single-purpose helper whose only
consumer is `CatalogManager`, so the seven validate-no-duplicates tests sit
naturally alongside the existing `serialize/deserialize` cases in the sibling
suite. PR description corrected to match. Happy to re-extract if you'd prefer
the strict source-mirror pattern.
- **"Explicitly deferred" claim about the single-pass resolver** — you were
right, my claim was stale.
`FunctionResolverUtils.isUnqualifiedCountShadowedByTemp` IS live and wired into
`isNonDistinctCount` / `handleStarInArguments`; only the
`spark.sql.analyzer.singlePassResolver.enabled` conf default keeps the path
dormant in CI. Added a parallel `SetPathSuite` test that enables that conf for
the SELECT only (CREATE TEMPORARY FUNCTION and SET PATH stay on the fixed-point
analyzer because the single-pass analyzer doesn't yet support those operators)
and asserts the analyzed-plan output dataType (BIGINT when the shortcut fires,
INT when the gate suppresses it and the temp `count(INT) RETURN x + 100` wins).
Bypassing `checkAnswer` avoids the `DeserializeToObject` operator the
single-pass analyzer also doesn't yet support, but the gate predicate is
exercised end-to-end through the analyzer. The "deferred" bullet for this item
has been removed from the PR description.
--
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]
