ronantakizawa/codereview-bench huggingface dataset:
Repository: ClickHouse/ClickHouse
File: tests/queries/0_stateless/03800_autopr_reuse_index_analysis.sql (SQL)

Code before:
-- Tags: stateful, long

SET enable_parallel_replicas=0, automatic_parallel_replicas_mode=1, parallel_replicas_local_plan=1, parallel_replicas_index_analysis_only_on_coordinator=1,
    parallel_replicas_for_non_replicated_merge_tree=1, max_parallel_replicas=3, cluster_for_parallel_replicas='parallel_replicas';

SET automatic_parallel_replicas_min_bytes_per_replica=0;

SET use_query_condition_cache=0;

-- Check that index analysis is performed only once in both cases: when we choose local plan and when we choose parallel replicas

-- Pre-warm the cache
SELECT URL FROM test.hits WHERE UserID >= 730800628386 FORMAT Null;

SELECT sum(length(URL)) FROM test.hits WHERE UserID >= 730800628386 FORMAT Null;

-- Local plan wins
SELECT URL FROM test.hits WHERE UserID >= 730800628386 FORMAT Null SETTINGS log_comment='query_1';

-- Parallel replicas plan wins
SELECT sum(length(URL)) FROM test.hits WHERE UserID >= 730800628386 FORMAT Null SETTINGS log_comment='query_2';

-- A case when subquery is present

create table t(a UInt64) engine=MergeTree order by a;
insert into t select number from numbers_mt(1e6);

-- Pre-warm the cache

Review comment (suggestion):
`parallel_replicas_index_analysis_only_on_coordinator`=1 is not randomized, so, in general, no need to set explicitly

Code after:
-- Tags: stateful, long

SET enable_parallel_replicas=0, automatic_parallel_replicas_mode=1, parallel_replicas_local_plan=1, parallel_replicas_index_analysis_only_on_coordinator=1,
    parallel_replicas_for_non_replicated_merge_tree=1, max_parallel_replicas=3, cluster_for_parallel_replicas='parallel_replicas';

-- To avoid too slow test execution
set remote_filesystem_read_method='threadpool', allow_prefetched_read_pool_for_remote_filesystem=1, filesystem_prefetch_step_marks=0, filesystem_prefetch_step_bytes='100Mi';

SET automatic_parallel_replicas_min_bytes_per_replica=0;

SET use_query_condition_cache=0;

-- Check that index analysis is performed only once in both cases: when we choose local plan and when we choose parallel replicas

-- Pre-warm the cache
SELECT URL FROM test.hits WHERE UserID >= 730800628386 FORMAT Null;

SELECT sum(length(URL)) FROM test.hits WHERE UserID >= 730800628386 FORMAT Null;

-- Local plan wins
SELECT URL FROM test.hits WHERE UserID >= 730800628386 FORMAT Null SETTINGS log_comment='3800_autopr_reuse_index_analysis_query_1';

-- Parallel replicas plan wins
SELECT sum(length(URL)) FROM test.hits WHERE UserID >= 730800628386 FORMAT Null SETTINGS log_comment='3800_autopr_reuse_index_analysis_query_2';

-- A case when subquery is present

create table t(a UInt64) engine=MergeTree order by a;
Repository: ClickHouse/ClickHouse
File: src/Common/AggregatedMetrics.h (C/C++)

Code before:
#pragma once

#include <Common/CurrentMetrics.h>

#include <atomic>

/** Allows to aggregate partially changing values into single CurrentMetric.
  * Destination CurrentMetric will be updated instantly after changes will be applied to handle.
  */
namespace AggregatedMetrics
{

class GlobalSum
{
public:
    explicit GlobalSum(CurrentMetrics::Metric destination_metric_) noexcept;
    ~GlobalSum() noexcept;

    void set(CurrentMetrics::Value value) noexcept;
    void add(CurrentMetrics::Value delta) noexcept;
    void sub(CurrentMetrics::Value delta) noexcept;

private:
    const CurrentMetrics::Metric destination_metric;
    alignas(64) std::atomic<CurrentMetrics::Value> accounted_value = 0;
};

class GlobalQuantile
{
public:
    explicit GlobalQuantile(CurrentMetrics::Metric destination_metric_) noexcept;
    ~GlobalQuantile() noexcept;

    void set(CurrentMetrics::Value value) noexcept;

private:
    const CurrentMetrics::Metric destination_metric;

    double quantile;
    void * shared_buckets = nullptr;
    void * shared_update = nullptr;

    bool was_accounted = false;
    alignas(64) std::atomic<CurrentMetrics::Value> accounted_value = 0;
};

}


Review comment (suggestion):
```suggestion
    std::atomic<CurrentMetrics::Value> accounted_value = 0;
```

Likely can be removed after https://github.com/ClickHouse/ClickHouse/pull/96097

Code after:
#pragma once

#include <Common/CurrentMetrics.h>

#include <atomic>

/** Allows to aggregate partially changing values into single CurrentMetric.
  * Destination CurrentMetric will be updated instantly after changes will be applied to handle.
  */
namespace AggregatedMetrics
{

class GlobalSum
{
public:
    explicit GlobalSum(CurrentMetrics::Metric destination_metric_) noexcept;
    ~GlobalSum() noexcept;

    void set(CurrentMetrics::Value value) noexcept;
    void add(CurrentMetrics::Value delta) noexcept;
    void sub(CurrentMetrics::Value delta) noexcept;

private:
    const CurrentMetrics::Metric destination_metric;
    std::atomic<CurrentMetrics::Value> accounted_value = 0;
};

class GlobalQuantile
{
public:
    explicit GlobalQuantile(CurrentMetrics::Metric destination_metric_) noexcept;
    ~GlobalQuantile() noexcept;

    void set(CurrentMetrics::Value value) noexcept;

private:
    const CurrentMetrics::Metric destination_metric;

    double quantile;
    void * shared_buckets = nullptr;
    void * shared_update = nullptr;

    bool was_accounted = false;
    std::atomic<CurrentMetrics::Value> accounted_value = 0;
};

}
Repository: ClickHouse/ClickHouse
File: src/Interpreters/ReplicatedDatabaseQueryStatusSource.h (C/C++)

Code before:
#include <Interpreters/DistributedQueryStatusSource.h>
#include <Common/ZooKeeper/ZooKeeperRetries.h>

namespace DB
{
class ReplicatedDatabaseQueryStatusSource final : public DistributedQueryStatusSource
{
public:
    ReplicatedDatabaseQueryStatusSource(
        const String & zk_node_path, const String & zk_replicas_path, ContextPtr context_, const Strings & hosts_to_wait, DDLGuardPtr && database_guard_);

    String getName() const override { return "ReplicatedDatabaseQueryStatus"; }

protected:
    ExecutionStatus checkStatus(const String & host_id) override;
    Chunk generateChunkWithUnfinishedHosts() const override;
    Strings getNodesToWait() override;
    Chunk handleTimeoutExceeded() override;
    Chunk stopWaitingOfflineHosts() override;
    void handleNonZeroStatusCode(const ExecutionStatus & status, const String & host_id) override;
    void fillHostStatus(const String & host_id, const ExecutionStatus & status, MutableColumns & columns) override;

private:
    static Block getSampleBlock();
    DDLGuardPtr database_guard;


};
}


Review comment (suggestion):
```suggestion
    /// A kind of read lock for the database which prevents dropping the database (and its metadata from zk that we use for getting the query status)
    DDLGuardPtr database_guard;
```

Code after:
#include <Interpreters/DistributedQueryStatusSource.h>
#include <Common/ZooKeeper/ZooKeeperRetries.h>

namespace DB
{
class ReplicatedDatabaseQueryStatusSource final : public DistributedQueryStatusSource
{
public:
    ReplicatedDatabaseQueryStatusSource(
        const String & zk_node_path, const String & zk_replicas_path, ContextPtr context_, const Strings & hosts_to_wait, DDLGuardPtr && database_guard_);

    String getName() const override { return "ReplicatedDatabaseQueryStatus"; }

protected:
    ExecutionStatus checkStatus(const String & host_id) override;
    Chunk generateChunkWithUnfinishedHosts() const override;
    Strings getNodesToWait() override;
    Chunk handleTimeoutExceeded() override;
    Chunk stopWaitingOfflineHosts() override;
    void handleNonZeroStatusCode(const ExecutionStatus & status, const String & host_id) override;
    void fillHostStatus(const String & host_id, const ExecutionStatus & status, MutableColumns & columns) override;

private:
    static Block getSampleBlock();
    /// A kind of read lock for the database which prevents dropping the database (and its metadata from zk that we use for getting the query status)
    DDLGuardPtr database_guard;


};
}
Repository: ClickHouse/ClickHouse
File: tests/queries/0_stateless/03820_kusto_functions_empty_argument_validation.sql (SQL)

Code before:
-- Tags: no-fasttest
-- Test for issue #95509: Kusto functions should validate empty arguments properly
-- This test ensures that functions bin(), bin_at(), extract(), and indexof() 
-- throw proper exceptions instead of crashing when given empty arguments.

SET allow_experimental_kusto_dialect = 1;
SET dialect = 'kusto';

-- Test bin() function with empty bin size argument
-- Expected: Should throw BAD_ARGUMENTS exception, not crash
SELECT bin(4.5,,); -- { serverError BAD_ARGUMENTS }

-- Test bin_at() function with empty bin size argument  
-- Expected: Should throw BAD_ARGUMENTS exception, not crash
SELECT bin_at(datetime, 10.5,, 5.0); -- { serverError BAD_ARGUMENTS }

-- Test extract() function with empty capture group argument
-- Expected: Should throw BAD_ARGUMENTS exception, not crash
SELECT extract("User: ([^,]+)",, "User: James, Email: James@example.com, Age: 29"); -- { serverError BAD_ARGUMENTS }

-- Test indexof() function with empty start_index argument
-- Expected: Should throw BAD_ARGUMENTS exception, not crash
SELECT indexof("hello world", "world",,); -- { serverError BAD_ARGUMENTS }

-- Test indexof() function with empty length argument
-- Expected: Should throw BAD_ARGUMENTS exception, not crash
SELECT indexof("hello world", "world", 0,,); -- { serverError BAD_ARGUMENTS }

-- Test indexof() function with empty occurrence argument
-- Expected: Should throw BAD_ARGUMENTS exception, not crash
SELECT indexof("hello world", "world", 0, 11,,); -- { serverError BAD_ARGUMENTS }

-- Verify that functions still work correctly with valid arguments
SELECT bin(4.5, 2.0);
SELECT extract("User: ([^,]+)", 1, "User: James, Email: James@example.com, Age: 29");
SELECT indexof("hello world", "world", 0);

Review comment (suggestion):
I'm not sure how you got it to work to produce .reference, but it shouldn't - you need to rewrite it in KQL syntax

Code after:

SET allow_experimental_kusto_dialect = 1;
SET dialect = 'kusto';

print bin(, 1.5); -- { clientError SYNTAX_ERROR }
print bin_at(, 2.5, 7); -- { clientError SYNTAX_ERROR }
print iif(, 1, 2); -- { clientError SYNTAX_ERROR }
print indexof('abc', , 0); -- { clientError SYNTAX_ERROR }
print extract("User: ([^,]+)", , "User: James, Email: James@example.com, Age: 29"); -- { clientError SYNTAX_ERROR }
Repository: ClickHouse/ClickHouse
File: tests/queries/0_stateless/03821_check_access_for_system_tables_performance.sh (Shell)

Code before:

NUM_DATABASES=300
user="user_03821_$CLICKHOUSE_DATABASE"

function cleanup()
{
    for i in $(seq 1 $NUM_DATABASES); do
        $CLICKHOUSE_CLIENT --query "DROP DATABASE IF EXISTS test_03821_db_$i" 2>/dev/null &
    done
    $CLICKHOUSE_CLIENT --query "DROP USER IF EXISTS $user" 2>/dev/null
}
trap cleanup EXIT

for i in $(seq 1 $NUM_DATABASES); do
    $CLICKHOUSE_CLIENT --query "CREATE DATABASE IF NOT EXISTS test_03821_db_$i" &
done

$CLICKHOUSE_CLIENT --query "DROP USER IF EXISTS $user"
$CLICKHOUSE_CLIENT --query "CREATE USER $user"
$CLICKHOUSE_CLIENT --query "GRANT SHOW DATABASES ON test_03821_db_1.* TO $user"
$CLICKHOUSE_CLIENT --query "GRANT SHOW DATABASES ON test_03821_db_2.* TO $user"
$CLICKHOUSE_CLIENT --query "GRANT SHOW DATABASES ON test_03821_db_3.* TO $user"
$CLICKHOUSE_CLIENT --query "GRANT SHOW DATABASES ON test_03821_db_4.* TO $user"
$CLICKHOUSE_CLIENT --query "GRANT SHOW DATABASES ON test_03821_db_5.* TO $user"

$CLICKHOUSE_CLIENT --user "$user" --query "SELECT count() FROM system.databases WHERE name LIKE 'test_03821_db_%'"


Review comment (suggestion):
As we only want to match `_` and not as a wildcard. It probably doesn't cause any issues, but let's be pedantic.
```suggestion
$CLICKHOUSE_CLIENT --user "$user" --query "SELECT count() FROM system.databases WHERE name LIKE 'test\_03821\_db\_%'"
```

Code after:

NUM_DATABASES=300
user="user_03821_$CLICKHOUSE_DATABASE"

# Build PARALLEL WITH compound queries for create/drop
query_create=""
query_drop=""
for i in $(seq 1 $NUM_DATABASES); do
    if [ -n "$query_create" ]; then
        query_create+=" PARALLEL WITH "
        query_drop+=" PARALLEL WITH "
    fi
    query_create+="CREATE DATABASE IF NOT EXISTS test_03821_db_$i ENGINE=Memory"
    query_drop+="DROP DATABASE IF EXISTS test_03821_db_$i"
done

function cleanup()
{
    $CLICKHOUSE_CLIENT --query "$query_drop" 2>/dev/null
    $CLICKHOUSE_CLIENT --query "DROP USER IF EXISTS $user" 2>/dev/null
}
trap cleanup EXIT

$CLICKHOUSE_CLIENT --query "$query_create"

$CLICKHOUSE_CLIENT --query "DROP USER IF EXISTS $user"
$CLICKHOUSE_CLIENT --query "CREATE USER $user"
$CLICKHOUSE_CLIENT --query "GRANT SHOW DATABASES ON test_03821_db_1.*, SHOW DATABASES ON test_03821_db_2.*, SHOW DATABASES ON test_03821_db_3.*, SHOW DATABASES ON test_03821_db_4.*, SHOW DATABASES ON test_03821_db_5.* TO $user"

$CLICKHOUSE_CLIENT --user "$user" --query "SELECT count() FROM system.databases WHERE name LIKE 'test\_03821\_db\_%'"
Repository: ClickHouse/ClickHouse
File: src/Storages/System/StorageSystemPartsBase.cpp (C++)

Code before:
    {
        if (!columns.has(column_name))
            return;
        ColumnDescription column(alias_name, columns.get(column_name).type);
        column.default_desc.kind = ColumnDefaultKind::Alias;
        column.default_desc.expression = make_intrusive<ASTIdentifier>(column_name);
        columns.add(column);
    };

    /// Add aliases for old column names for backwards compatibility.
    add_alias("bytes", "bytes_on_disk");
    add_alias("marks_size", "marks_bytes");
    add_alias("part_name", "name");

    StorageInMemoryMetadata storage_metadata;
    storage_metadata.setColumns(columns);
    setInMemoryMetadata(storage_metadata);

    VirtualColumnsDescription virtuals;
    virtuals.addEphemeral("_state", std::make_shared<DataTypeString>(), "");
    setVirtuals(std::move(virtuals));
}

bool StoragesInfoStreamBase::tryLockTable(StoragesInfo & info)
{
    info.table_lock = info.storage->tryLockForShare(query_id, lock_timeout);
    // nullptr means table was dropped while acquiring the lock
    return info.table_lock != nullptr;
}
}


Review comment (bug):
The repeated pattern `Poco::Timespan(lock_timeout.count() * 1000)` is easy to get wrong (units/overflow) and is now duplicated across multiple call sites. Prefer storing the timeout as `Poco::Timespan` end-to-end (since the setting is already a timespan), or introduce a small helper for `std::chrono::milliseconds -> Poco::Timespan` conversion to centralize the unit logic.

Code after:
    {
        if (!columns.has(column_name))
            return;
        ColumnDescription column(alias_name, columns.get(column_name).type);
        column.default_desc.kind = ColumnDefaultKind::Alias;
        column.default_desc.expression = make_intrusive<ASTIdentifier>(column_name);
        columns.add(column);
    };

    /// Add aliases for old column names for backwards compatibility.
    add_alias("bytes", "bytes_on_disk");
    add_alias("marks_size", "marks_bytes");
    add_alias("part_name", "name");

    StorageInMemoryMetadata storage_metadata;
    storage_metadata.setColumns(columns);
    setInMemoryMetadata(storage_metadata);

    VirtualColumnsDescription virtuals;
    virtuals.addEphemeral("_state", std::make_shared<DataTypeString>(), "");
    setVirtuals(std::move(virtuals));
}

bool StoragesInfoStreamBase::tryLockTable(StoragesInfo & info)
{
    info.table_lock = info.storage->tryLockForShare(query_id, Poco::Timespan(lock_timeout.count() * 1000));
    // nullptr means table was dropped while acquiring the lock
    return info.table_lock != nullptr;
}
}
Repository: ClickHouse/ClickHouse
File: tests/queries/0_stateless/03829_distance_transposed_pass_nullable_nothing.sql (SQL)

Code before:
-- Regression test: DistanceTransposedPartialReadsPass should not throw a logical error
-- when the function result type is Nullable(Nothing) due to NULL arguments.
-- https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=6d880459c77dd2082259ce909163c64f8cce6f22&name_0=MasterCI&name_1=AST%20fuzzer%20%28amd_ubsan%29

DROP TABLE IF EXISTS qbit_test;
CREATE TABLE qbit_test (id UInt64, vec QBit(BFloat16, 18)) ENGINE = Memory;
INSERT INTO qbit_test VALUES (1, [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18]);

SELECT L2DistanceTransposed(vec, NULL, 32) FROM qbit_test;

DROP TABLE qbit_test;


Review comment (suggestion):
```suggestion
CREATE TABLE qbit_test (id UInt64, vec QBit(Float32, 18)) ENGINE = Memory;
```

it doesn't reproduce with `BFloat16`

Code after:
-- Regression test: DistanceTransposedPartialReadsPass should not throw a logical error
-- when the function result type is Nullable(Nothing) due to NULL arguments.
-- https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=6d880459c77dd2082259ce909163c64f8cce6f22&name_0=MasterCI&name_1=AST%20fuzzer%20%28amd_ubsan%29

DROP TABLE IF EXISTS qbit_test;
CREATE TABLE qbit_test (id UInt64, vec QBit(Float32, 18)) ENGINE = Memory;
INSERT INTO qbit_test VALUES (1, [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18]);

SELECT L2DistanceTransposed(vec, NULL, 32) FROM qbit_test;

DROP TABLE qbit_test;
Repository: ClickHouse/ClickHouse
File: tests/integration/test_storage_iceberg_with_spark/test_writes_create_table.py (Python)

Code before:
        ("(c0 Int)", "identity(1+1)"),
        ("(c0 Int)", "identity()"),
        ("(c0 Int, c1 Int)", "identity(c0, c1, c0)"),
        ("(c0 Int, c1 Int)", "icebergBucket(c0, c1)"),
        ("(c0 Int, c1 Int)", "icebergBucket(1, 2)"),
        ("(c0 Int)", "icebergBucket(c0, 1)"),
        ("(c0 Int)", "icebergBucket(-1, c0)"),
        ("(c0 Int)", "icebergTruncate(1.5, c0)"),
    ]
)
def test_order_by_bad_arguments(
    started_cluster_iceberg_with_spark, format_version, storage_type, schema, order_by
):
    instance = started_cluster_iceberg_with_spark.instances["node1"]
    table_name = "test_order_by_bad_" + storage_type + "_" + get_uuid_str()

    with pytest.raises(Exception):
        create_iceberg_table(
            storage_type,
            instance,
            table_name,
            started_cluster_iceberg_with_spark,
            schema,
            format_version=format_version,
            order_by=order_by,
        )


Review comment (suggestion):
Please, use function node.query_and_get_error and check that you indeed have expected error text for each call (and BAD_ARGUMENTS call)

Code after:
        ("(c0 Int)", "identity(1)"),
        ("(c0 Int)", "identity(1+1)"),
        ("(c0 Int, c1 Int)", "icebergBucket(c0, c1)"),
        ("(c0 Int, c1 Int)", "icebergBucket(1, 2)"),
        ("(c0 Int)", "icebergBucket(c0, 1)"),
        ("(c0 Int)", "icebergBucket(-1, c0)"),
    ]
)
def test_order_by_bad_arguments_ast_parsing(
    started_cluster_iceberg_with_spark, format_version, storage_type, schema, order_by
):
    instance = started_cluster_iceberg_with_spark.instances["node1"]
    table_name = "test_order_by_bad_ast_" + storage_type + "_" + get_uuid_str()

    query = get_creation_expression(
        storage_type,
        table_name,
        started_cluster_iceberg_with_spark,
        schema,
        format_version=format_version,
        order_by=order_by,
    )
    
    error = instance.query_and_get_error(query)
    assert (
        "Code: 36" in error or "BAD_ARGUMENTS" in error
    ), f"Expected BAD_ARGUMENTS error (Code: 36), got: {error}"
    assert (
        "Invalid iceberg sort order" in error 
        or "Unsupported function" in error 
        or "expected a column identifier" in error.lower()
        or "expected (integer_literal, column_identifier)" in error.lower()
        or "expected 1 or 2 arguments" in error.lower()
        or "expected a non-negative integer literal" in error.lower()
        or "arguments are missing" in error.lower()
    ), f"Expected error message about invalid sort order, got: {error}"


@pytest.mark.parametrize("format_version", [2])
@pytest.mark.parametrize("storage_type", ["s3"])
@pytest.mark.parametrize(
    "schema,order_by",
    [
        ("(c0 Int)", "identity()"),
        ("(c0 Int, c1 Int)", "identity(c0, c1, c0)"),
    ]
)
def test_order_by_bad_arguments_wrong_count(
    started_cluster_iceberg_with_spark, format_version, storage_type, schema, order_by
):
    instance = started_cluster_iceberg_with_spark.instances["node1"]
Repository: ClickHouse/ClickHouse
File: tests/queries/0_stateless/03827_cross_join_no_right_columns.sql (SQL)

Code before:
-- Test that cross join works correctly when no columns from the right side are needed.
-- This used to cause std::out_of_range in HashJoin::getTotalRowCount() because
-- columns_info.columns was empty but .at(0) was used to get the row count.
-- The bug triggers when PREWHERE consumes ALL columns from the right table,
-- causing the right side of the join to pass zero columns to HashJoin.

DROP TABLE IF EXISTS t1;
DROP TABLE IF EXISTS t2;

CREATE TABLE t1 (x UInt64) ENGINE = MergeTree ORDER BY x;
CREATE TABLE t2 (a UInt64, b String) ENGINE = MergeTree ORDER BY a;

INSERT INTO t1 VALUES (1), (2), (3);
INSERT INTO t2 VALUES (1, 'x'), (2, 'y');

-- PREWHERE references all columns from t2, so after PREWHERE pushdown
-- the right side of the join has zero columns in its header.
SELECT count() FROM t1, t2 PREWHERE a > 0 AND b != '';

DROP TABLE t1;
DROP TABLE t2;


Review comment (bug):
Test fails with old analyzer, which does not support PREWHERE for right joined table and expect to have only left columns there.

```suggestion
SET enable_analyzer = 1;
```

Code after:
-- Test that cross join works correctly when no columns from the right side are needed.
-- This used to cause std::out_of_range in HashJoin::getTotalRowCount() because
-- columns_info.columns was empty but .at(0) was used to get the row count.
-- The bug triggers when PREWHERE consumes ALL columns from the right table,
-- causing the right side of the join to pass zero columns to HashJoin.

DROP TABLE IF EXISTS t1;
DROP TABLE IF EXISTS t2;

CREATE TABLE t1 (x UInt64) ENGINE = MergeTree ORDER BY x;
CREATE TABLE t2 (a UInt64, b String) ENGINE = MergeTree ORDER BY a;

INSERT INTO t1 VALUES (1), (2), (3);
INSERT INTO t2 VALUES (1, 'x'), (2, 'y');
SET enable_analyzer = 1;
-- PREWHERE references all columns from t2, so after PREWHERE pushdown
-- the right side of the join has zero columns in its header.
SELECT count() FROM t1, t2 PREWHERE a > 0 AND b != '';

DROP TABLE t1;
DROP TABLE t2;
Repository: ClickHouse/ClickHouse
File: tests/queries/0_stateless/03643_system_instrumentation_no_suspicious_lowcardinality.sql (SQL)

Code before:
-- Tags: no-fasttest
-- fasttest: Instrumentation requires XRAY
-- Test that system.instrumentation.function_id is Int32, not LowCardinality(Int32)
-- This allows CREATE VIEW without allow_suspicious_low_cardinality_types setting

-- Verify the column type
SELECT name, type FROM system.columns WHERE database = 'system' AND table = 'instrumentation' AND name = 'function_id';

-- This should work without setting allow_suspicious_low_cardinality_types
DROP VIEW IF EXISTS test_instrumentation_view;
CREATE VIEW test_instrumentation_view AS
SELECT
    id,
    function_id,
    function_name,
    handler,
    entry_type,
    symbol,
    parameters
FROM system.instrumentation;

-- Verify the view was created
SELECT name FROM system.tables WHERE database = currentDatabase() AND name = 'test_instrumentation_view';

-- Clean up
DROP VIEW test_instrumentation_view;


Review comment (suggestion):
```suggestion
-- no-fasttest: Instrumentation requires XRay
```

Code after:
-- Tags: use-xray
-- Test that system.instrumentation.function_id is Int32, not LowCardinality(Int32)
-- This allows CREATE VIEW without allow_suspicious_low_cardinality_types setting

-- Verify the column type
SELECT name, type FROM system.columns WHERE database = 'system' AND table = 'instrumentation' AND name = 'function_id';

-- This should work without setting allow_suspicious_low_cardinality_types
DROP VIEW IF EXISTS test_instrumentation_view;
CREATE VIEW test_instrumentation_view AS
SELECT
    id,
    function_id,
    function_name,
    handler,
    entry_type,
    symbol,
    parameters
FROM system.instrumentation;

-- Verify the view was created
SELECT name FROM system.tables WHERE database = currentDatabase() AND name = 'test_instrumentation_view';

-- Clean up
DROP VIEW test_instrumentation_view;