[SPARK-52388][SQL] Handle named and positional parameters under `PlanWithUnresolvedIdentifier`

[mihailotim-db]

What changes were proposed in this pull request?

Handle named and positional parameters when they appear under PlanWithUnresolvedIdentifier.

Why are the changes needed?

For a query like:

INSERT INTO IDENTIFIER(:a)
REPLACE WHERE col1 = :b
SELECT * FROM VALUES(3) as t(col1)

Unresolved plan is going to be:

'NameParameterizedQuery [a, b], [testcat.schema.testtbl, true]
+- 'PlanWithUnresolvedIdentifier namedparameter(a), org.apache.spark.sql.catalyst.parser.AstBuilder$$Lambda$3153/0x000000e801e96788@58c36104
   +- 'Project [*]
      +- SubqueryAlias t
         +- LocalRelation [col1#20]

Because namedParameter(b) is going to be hidden in the planBuilder of PlanWithUnresolvedIdentifier, BindParamters is only going to be able to bind parameter a. However, current implementations is going to completely remove NamedParametrizedQuery node even thought there are still unbound parameters left.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add a new test to verify that the query pattern is no longer failing.

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

No

[cloud-fan]

My opinion is that PlanWithUnresolvedIdentifier should hide as few things as possible. We should change PlanWithUnresolvedIdentifier(... => InsertIntoStatement) to InsertIntoStatement(PlanWithUnresolvedIdentifier(... => UnresolvedRelation))

[mihailotim-db]

My opinion is that PlanWithUnresolvedIdentifier should hide as few things as possible. We should change PlanWithUnresolvedIdentifier(... => InsertIntoStatement) to InsertIntoStatement(PlanWithUnresolvedIdentifier(... => UnresolvedRelation))

I don't like the idea of implementing a fix for a specific operator, mostly because we are bound to find more issues like this. For example, in this case we would fail for any query that has this pattern of a parameter inside PlanWithUnresolvedIdentifier.

[cloud-fan]

Hiding plans/expressions with a lambda function is evil. We should do the right thing even if it's not easy (need to fix commands one by one).

[cloud-fan]

I don't believe this is the right fix. We should fix every place in the parser to not hide plans/expressions within a lambda function.

[github-actions[bot]]

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!