Support transformQueryForExternalDatabase for analyzer by vdimir · Pull Request #47316 · ClickHouse/ClickHouse

vdimir · 2023-03-07T20:43:48Z

Changelog category (leave one):

Not for changelog (changelog entry is not required)

~~This PR contains commits from #45461 need to remove them and leave only new ones before merging~~

Basic support transformQueryForExternalDatabase with converting query tree to ast
Support SETTINGS argument of table functions
Support transformQueryForExternalDatabase for query tree natively without reusing ast-based code.

vdimir · 2023-03-10T09:48:27Z

Compare Integration Tests (release) results in ffdbdf7 ("Analyzer Planner enable by default") and 1908c38 (current PR)

List of tests that are fixed by this patch (see in ci database):

List

test_dictionaries_postgresql/test.py::test_invalidate_query
test_dictionaries_postgresql/test.py::test_load_dictionaries
test_odbc_interaction/test.py::test_concurrent_queries
test_odbc_interaction/test.py::test_many_connections
test_odbc_interaction/test.py::test_mysql_insert
test_odbc_interaction/test.py::test_mysql_simple_select_works
test_odbc_interaction/test.py::test_odbc_cyrillic_with_varchar
test_odbc_interaction/test.py::test_odbc_long_column_names
test_odbc_interaction/test.py::test_odbc_long_text
test_odbc_interaction/test.py::test_odbc_postgres_conversions
test_odbc_interaction/test.py::test_odbc_postgres_date_data_type
test_odbc_interaction/test.py::test_sqlite_simple_select_function_works
test_odbc_interaction/test.py::test_sqlite_simple_select_storage_works
test_odbc_interaction/test.py::test_sqlite_table_function
test_postgresql_replica_database_engine_2/test.py::test_add_new_table_to_replication
test_postgresql_replica_database_engine_2/test.py::test_database_with_multiple_non_default_schemas_1
test_postgresql_replica_database_engine_2/test.py::test_database_with_multiple_non_default_schemas_2
test_postgresql_replica_database_engine_2/test.py::test_database_with_single_non_default_schema
test_postgresql_replica_database_engine_2/test.py::test_materialized_view
test_postgresql_replica_database_engine_2/test.py::test_predefined_connection_configuration
test_postgresql_replica_database_engine_2/test.py::test_table_override
test_storage_mysql/test.py::test_binary_type
test_storage_mysql/test.py::test_enum_type
test_storage_mysql/test.py::test_external_settings
test_storage_mysql/test.py::test_insert_on_duplicate_select
test_storage_mysql/test.py::test_insert_select
test_storage_mysql/test.py::test_many_connections
test_storage_mysql/test.py::test_mysql_distributed
test_storage_mysql/test.py::test_mysql_in
test_storage_mysql/test.py::test_mysql_null
test_storage_mysql/test.py::test_predefined_connection_configuration
test_storage_mysql/test.py::test_replace_select
test_storage_mysql/test.py::test_settings_connection_wait_timeout
test_storage_mysql/test.py::test_table_function
test_storage_mysql/test.py::test_where

Query

WITH failed_before_fix AS (SELECT test_name FROM checks
    WHERE test_status not in ('OK', 'SKIPPED')
        AND check_name ilike '%integra%'
        AND pull_request_number = 45461
        AND commit_sha = 'ffdbdf79c5444c9bd4418e624225622cd723cc86'
)
SELECT
    test_name,
    substring(check_name, 1, 15) || '...' || substring(check_name, -15) as check,
    check_start_time,
    test_status,
    substring(commit_sha, 1, 10) as commit_short,
    report_url,
    pull_request_number as prnum
FROM checks
WHERE test_status in ('OK', 'SKIPPED')
    AND now() - INTERVAL '10' day <= check_start_time
    AND check_name ilike '%integra%release%'
    AND pull_request_number = 47316
    AND commit_sha = '1908c38c739a46051feda356df01711396ceb520'
    AND test_name in failed_before_fix
ORDER BY test_name
LIMIT 1 BY test_name

Tests that are still failing: ci database

It contains some tests for mysql, sqlite or postgresql, I will work on them.

vdimir · 2023-03-13T14:20:47Z

AST fuzzer (asan) — Logical error: 'Trying to get name of not a column: Set'. Details

AST fuzzer (ubsan) — Logical error: 'Trying to get name of not a column: Set'. Details

Not related to analyzer, reproduces on master. Seems like just missing check, but not sure how to fix with minimum of changes (without changing IAST::appendColumnName or introducing new method).

#47533

src/Analyzer/IQueryTreeNode.h

vdimir · 2023-03-16T11:17:13Z

Stateless tests (release) — fail: 1, passed: 5113, skipped: 4 Details

02417_opentelemetry_insert_on_distributed_table failures in ci db

Stateless tests (tsan) [2/5] — fail: 1, passed: 990, skipped: 5 Details

01075_window_view_proc_tumble_to_now_populate failures in ci db

src/Analyzer/ArrayJoinNode.cpp

src/Analyzer/ColumnNode.h

src/Storages/transformQueryForExternalDatabaseAnalyzer.cpp

tests/queries/0_stateless/02479_mysql_connect_to_self.reference

kitaisreal · 2023-03-22T09:00:52Z

src/Interpreters/InterpreterSelectQueryAnalyzer.h


+    const QueryTreeNodePtr & getQueryTree() const { return query_tree; }
+
+    SelectQueryInfo getSelectQueryInfo()


We should not introduce such method, because client can thing that this select query info can be used somewhere, but each table expression get unique select query info.
If this is only necessary for tests, maybe we can find some workaround ?

~~Added friend struct from unit tests~~
UPD: implemented another approach to get it in test, added function buildSelectQueryInfo into Utils

src/Interpreters/InterpreterSelectQueryAnalyzer.h

src/Analyzer/TableFunctionNode.cpp

vdimir · 2023-03-23T10:02:27Z

Stateless tests (tsan) [1/5] — Tests are not finished, fail: 2, passed: 81 Details

WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=672)
https://s3.amazonaws.com/clickhouse-test-reports/47316/426f4ec233c4ff6bcb8943b2a34f19c4ab817116/stateless_tests__tsan__[1/5]/stderr.log

Issue #47460

kitaisreal · 2023-03-24T16:43:28Z

src/Storages/tests/gtest_transform_query_for_external_database.cpp

        context->setCurrentDatabase("test");
    }
+
+    DatabasePtr mockSystemDatabase()


This function seems unused.

vdimir added pr-not-for-changelog This PR should not be mentioned in the changelog force tests labels Mar 7, 2023

kitaisreal self-assigned this Mar 8, 2023

vdimir force-pushed the vdimir/transform_external_query_analyzer branch 8 times, most recently from 590e2b4 to 1908c38 Compare March 9, 2023 14:30

vdimir force-pushed the vdimir/transform_external_query_analyzer branch from 1908c38 to 6fc9f4d Compare March 13, 2023 10:46

vdimir changed the title ~~WIP transformQueryForExternalDatabase for analyzer~~ Support transformQueryForExternalDatabase for analyzer Mar 13, 2023

vdimir mentioned this pull request Mar 13, 2023

[Umbrella] Analyzer, Planner migration #42648

Closed

vdimir removed the force tests label Mar 13, 2023

kitaisreal suggested changes Mar 13, 2023

View reviewed changes

src/Analyzer/IQueryTreeNode.h Outdated Show resolved Hide resolved

vdimir marked this pull request as ready for review March 14, 2023 09:45

vdimir added 7 commits March 22, 2023 08:59

Support transformQueryForExternalDatabase for analyzer

023d14a

Support SETTINGS argument of table functions

ebd71f2

Adjust test_mysql_protocol and others for analyzer

c2f0583

Fix 02479_mysql_connect_to_self

2537d0d

Pass ConvertToASTOptions by constref

a5cca40

Fix 02479_mysql_connect_to_self

26fdef4

Fix test_mysql_protocol

6919db3

vdimir force-pushed the vdimir/transform_external_query_analyzer branch from 06966ea to 6919db3 Compare March 22, 2023 07:59

kitaisreal suggested changes Mar 22, 2023

View reviewed changes

Better transformQueryForExternalDatabase for analyzer

426f4ec

vdimir requested a review from kitaisreal March 23, 2023 10:12

Add buildSelectQueryInfo to Planner/Utils, use in unit tests

c2a3679

kitaisreal reviewed Mar 24, 2023

View reviewed changes

kitaisreal approved these changes Mar 24, 2023

View reviewed changes

kitaisreal merged commit 5cb2d30 into master Mar 24, 2023

kitaisreal deleted the vdimir/transform_external_query_analyzer branch March 24, 2023 16:53


		const QueryTreeNodePtr & getQueryTree() const { return query_tree; }

		SelectQueryInfo getSelectQueryInfo()

Conversation

vdimir commented Mar 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Uh oh!

vdimir commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vdimir commented Mar 13, 2023

Uh oh!

Uh oh!

vdimir commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kitaisreal Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

vdimir Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vdimir commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kitaisreal Mar 24, 2023

Choose a reason for hiding this comment

Uh oh!

vdimir Mar 29, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vdimir commented Mar 7, 2023 •

edited

Loading

vdimir commented Mar 10, 2023 •

edited

Loading

vdimir commented Mar 16, 2023 •

edited

Loading

vdimir Mar 23, 2023 •

edited

Loading

vdimir commented Mar 23, 2023 •

edited

Loading