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.

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

order by sub stmt support db.table.col #6196

Merged
merged 2 commits into from
Jun 27, 2022

Conversation

TCeason
Copy link
Collaborator

@TCeason TCeason commented Jun 24, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

order by sub stmt support db.table.col.

In this pr just support db.table.col not do any judge for db_name and table_name.

because the struct ColumnBinding does not have database_name.

#[derive(Clone, PartialEq, Debug)]
pub struct ColumnBinding {
    /// Table name of this `ColumnBinding` in current context
    pub table_name: Option<String>,
    /// Column name of this `ColumnBinding` in current context
    pub column_name: String,
    /// Column index of ColumnBinding
    pub index: IndexType,

    pub data_type: DataTypeImpl,

    /// Consider the sql: `select * from t join t1 using(a)`.
    /// The result should only contain one `a` column.
    /// So we need make `t.a` or `t1.a` invisible in unqualified wildcard.
    pub visible_in_unqualified_wildcard: bool,
}

And actually I'm not sure this issue can be fixed as this pr.

In my mind it's better fixed with ISSUE #6080

Changelog

  • New Feature

Related Issues

Fixes #6192

@vercel
Copy link

vercel bot commented Jun 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Jun 27, 2022 at 4:20AM (UTC)

@TCeason TCeason requested a review from leiysky June 24, 2022 04:00
@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@mergify mergify bot added the pr-feature this PR introduces a new feature to the codebase label Jun 24, 2022
@TCeason TCeason marked this pull request as draft June 24, 2022 04:02
@TCeason TCeason force-pushed the ISSUE-6192/order_by_format branch 2 times, most recently from ff534c2 to 5e48a3e Compare June 24, 2022 09:46
@TCeason TCeason marked this pull request as ready for review June 24, 2022 09:47
-- ERROR 1105 (HY000): Code: 1006, displayText = Unable to get field named ""id2"_1". Valid fields: ["\"SUM(id)\"_4"].
SELECT SUM(id) as id2 FROM a.t ORDER BY a.t.id2;
-- expect err
SELECT DISTINCT(id) as id2 FROM a.t ORDER BY a.t.id2;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we need an error, but now it will succeed. I will create a new ISSUE.

'root'@mysqldb 17:57:14 [(none)]> SELECT DISTINCT(id) FROM a.t ORDER BY a.t.id2;
ERROR 1105 (HY000): Code: 1065, displayText = error: 
  --> SQL:1:39
  |
1 | SELECT DISTINCT(id) FROM a.t ORDER BY a.t.id2
  |                                       ^^^^^^^ for SELECT DISTINCT, ORDER BY expressions must appear in select list

@TCeason TCeason force-pushed the ISSUE-6192/order_by_format branch 3 times, most recently from 1e5f0af to ba8a891 Compare June 24, 2022 12:31
@TCeason TCeason marked this pull request as draft June 24, 2022 13:01
@TCeason
Copy link
Collaborator Author

TCeason commented Jun 24, 2022

@TCeason TCeason force-pushed the ISSUE-6192/order_by_format branch from ba8a891 to f622f2b Compare June 24, 2022 13:22
@BohuTANG
Copy link
Member

#6219 merged

@TCeason TCeason force-pushed the ISSUE-6192/order_by_format branch from f622f2b to 984aa15 Compare June 27, 2022 03:01
@TCeason TCeason marked this pull request as ready for review June 27, 2022 03:37
@TCeason TCeason requested a review from leiysky June 27, 2022 03:37
@@ -9,4 +9,5 @@ SCRIPT_PATH="$(cd "$(dirname "$0")" >/dev/null 2>&1 && pwd)"
cd "$SCRIPT_PATH/../../tests" || exit

echo "Starting databend-test"
./databend-test '^0[^4]_' --mode 'cluster' --run-dir 0_stateless --skip '02_0057_function_nullif' '02_0058_function_ifnull'
# Now Planner v2 not support cluster execute. So skip some tests that need enable planner v2.
./databend-test '^0[^4]_' --mode 'cluster' --run-dir 0_stateless --skip '02_0057_function_nullif' '02_0058_function_ifnull' '03_0004_select_order_by_db_table_col_v2'
Copy link
Collaborator Author

@TCeason TCeason Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the test in this pr needs to enable planner v2 but now we do not support cluster execute, so skip it.

@TCeason
Copy link
Collaborator Author

TCeason commented Jun 27, 2022

09_0008_fuse_optimize_table:                                            [ FAIL ] - result differs with:
--- /workspace/tests/suites/0_stateless/09_fuse_engine/09_0008_fuse_optimize_table.result	2022-06-27 04:37:36.977439331 +0000
+++ /workspace/tests/suites/0_stateless/09_fuse_engine/09_0008_fuse_optimize_table.stdout	2022-06-27 04:41:30.575939379 +0000
@@ -23,9 +23,3 @@
 9
 10
 1
-5
-6
-7
-8
-9
-10

Maybe that's a flaky test?

@BohuTANG BohuTANG merged commit 1450853 into databendlabs:main Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Orderby support use database_name.table_name.column_name
3 participants