Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MLv2] [BE] [Bug] returnedColumns does not handle columns with the same name #37517

Closed
Tracked by #36108
ranquild opened this issue Jan 10, 2024 · 1 comment · Fixed by #39118
Closed
Tracked by #36108

[MLv2] [BE] [Bug] returnedColumns does not handle columns with the same name #37517

ranquild opened this issue Jan 10, 2024 · 1 comment · Fixed by #39118
Assignees
@bshepherdson
Labels
.Backend .metabase-lib Label for tracking all issues related to the shared CLJC metabase-lib Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness Querying/MBQL .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Team/QueryProcessor :hammer_and_wrench:
Milestone
0.49.2

Comments

@ranquild
Copy link
Contributor

ranquild commented Jan 10, 2024 •
edited
Loading

name of the columns returned by returnedColumns does not match QP results. It's important for us that names of the columns generated by MLv2 match QP.

In this example nextColumnNames are generated by MLv2 and previousColumnNames are coming from query results (QP):

Screenshot 2024-01-10 at 18 24 34

Duplicate column names can be achieved e.g. by adding a self join to the query.
This was referenced Jan 10, 2024
Closed
Closed
Merged
@ranquild ranquild added .Backend .metabase-lib Label for tracking all issues related to the shared CLJC metabase-lib .Team/QueryProcessor :hammer_and_wrench: labels Jan 10, 2024
@metamben metamben self-assigned this Jan 11, 2024
metamben added a commit that referenced this issue Jan 12, 2024
@metamben
Introduce same-column-lists?
8055443 
Fixes #37517.

This function makes it possible for the FE to decide if the query has to be re-run.
@metamben metamben mentioned this issue Jan 12, 2024
Closed
@ranquild ranquild closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
@ranquild ranquild reopened this Feb 21, 2024
@ranquild ranquild modified the milestones: 0.48.7, 0.49 Feb 21, 2024
@ranquild ranquild assigned bshepherdson and unassigned metamben Feb 21, 2024
@ranquild
Copy link
Contributor Author

ranquild commented Feb 21, 2024 •
edited
Loading

This currently breaks all viz settings after question changes. Added "regression" label because it leads to regressions in the app behavior.

@ranquild ranquild added Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. labels Feb 21, 2024
bshepherdson added a commit that referenced this issue Feb 23, 2024
@bshepherdson
[MLv2] Ensure :name is unique on returned columns in the FE
1be0f86 
This is FE-only because this is a compatibility issue with the (legacy)
metadata coming back from the QP.

In time, the FE logic should be ported to MLv2 or made to depend on
things other than the `:name` field. MLv2 code itself (including BE
usage) should be based on the (already unique)
`:lib/desired-column-alias`.

Fixes #37517.
@bshepherdson bshepherdson mentioned this issue Feb 23, 2024
Merged
bshepherdson added a commit that referenced this issue Feb 23, 2024
@bshepherdson
[MLv2] Ensure :name is unique on returned columns in the FE (#39118)
4b80c77 
This is FE-only because this is a compatibility issue with the (legacy)
metadata coming back from the QP.

In time, the FE logic should be ported to MLv2 or made to depend on
things other than the `:name` field. MLv2 code itself (including BE
usage) should be based on the (already unique)
`:lib/desired-column-alias`.

Fixes #37517.
bshepherdson added a commit that referenced this issue Mar 26, 2024
@bshepherdson
backport "[MLv2] Ensure :name is unique on returned columns in the …
f257181 
…FE (#39118)"

Fixes #37517.
bshepherdson added a commit that referenced this issue Mar 26, 2024
@metabase-bot @bshepherdson
backport "[MLv2] Ensure :name is unique on returned columns in the …
f912df3 
…FE (#39118)" (#39122)

Fixes #37517.

Co-authored-by: Braden Shepherdson <braden@metabase.com>
@crisptrutski crisptrutski modified the milestones: 0.49, 0.49.2 Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bshepherdson bshepherdson

Labels
.Backend .metabase-lib Label for tracking all issues related to the shared CLJC metabase-lib Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness Querying/MBQL .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Team/QueryProcessor :hammer_and_wrench:
Projects
None yet
Milestone
0.49.2
5 participants
@perivamsi @bshepherdson @crisptrutski @ranquild @metamben