Revert "Preserve case for RowType's field name and JSON content when CAST"

[neeradsomanchi]

Reverts 5b7cda2

Description

This reverts the commit 5b7cda2, which was causing issues with user queries.

There is no flag covering the code introduced to check for duplicate field names. This causes some query failures because it breaks legacy behavior.

Test Plan

Unit Tests

Contributor checklist

• Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

[github-actions]

Codenotify: Notifying subscribers in CODENOTIFY files for diff 369f9d3...d20c599.

Notify	File(s)
@steveburnett	presto-docs/src/main/sphinx/admin/properties.rst presto-docs/src/main/sphinx/functions/json.rst

[mbasmanova]

CC: @tdcmeehan

[neeradsomanchi]

@hantangwangd Can you please cover the relevant logic (in the PR description) under a flag as well so that legacy behavior can be supported?

[steveburnett]

LGTM! (doc)

Pulled the branch and reviewed docs changes in a local docs build.

[hantangwangd]

@neeradsomanchi Sure. By the way, is this all the problems caused by the change? And could you provide some examples of the failed cases, so that I can add them to test cases for legacy behavior.

[hantangwangd]

@neeradsomanchi After recheck the code, I found it a little complex to cover the code in TypeSignature under a config flag. It may need a significant modification to pass the config flag. And I also wonder whether we should do this. Are the failed queries is something like follows and we want it to pass when the legacy support flag is true?

select CAST(ROW(1, 2) AS ROW(a BIGINT, A DOUBLE)).a;
select CAST(ROW(1, 2) AS ROW(KEY1 VARCHAR, "key1" VARCHAR));
select TYPEOF(CAST(row(1, 2) AS ROW(A VARCHAR, "a" VARCHAR)));
select TYPEOF(CAST(row(1, 2) AS ROW(a VARCHAR, A VARCHAR)));

If so, I think we should figure out firstly whether it is a incorrect behavior that need to be fixed even in legacy semantics. Any detailed messages would be appreciated!

cc: @tdcmeehan @rschlussel @mbasmanova

[rschlussel]

@hantangwangd, I checked the failures, and they were really user errors (seemed like typos) that we should have been failing all along. It was something like CAST (row(1, 'a') AS ROW(subfield BIGINT, subfield VARCHAR)) column. Before your change we only failed if you tried to use the duplicated field (e.g. if you then did SELECT column.subfield), with your change we fail even if you don't use the field.

Failing is the right thing to do here, and there isn't really a reasonable use case where users would use the old behavior on purpose, so it's fine if a few queries fail after this and users need to fix them. This should be good to add back as it was, but with a release note noting this fix.

[hantangwangd]

@rschlussel, thank you very much for the checking on failed user cases and the detailed informations. And so glad to know that this change do not affect normal existing user cases.

So, to add back this change, should we resubmit a PR the same as #21869 or revert the revert PR #22604? Which one do you think is the better way?

[rschlussel]

either is fine.