Skip to content

Add DateTime64 regression test for text/binary epoch equality (Fixes … #472

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

Open
ansh0014 wants to merge 1 commit into
base: master
Choose a base branch
from
+95 −29
Open

Add DateTime64 regression test for text/binary epoch equality (Fixes … #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 52 additions & 29 deletions tests/simple/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,43 @@ inline void DateTime64Example(Client& client) {
client.Execute("DROP TEMPORARY TABLE test_datetime64");
}

inline void DateTime64Issue398Check(Client& client) {
client.Execute("CREATE TEMPORARY TABLE test_datetime64_issue398 (dt64 DateTime64(6))");

// Text insert (server parses this in server timezone for DateTime64 without explicit timezone).
client.Execute("INSERT INTO test_datetime64_issue398 VALUES ('2024-10-10 11:00:00')");

// Ask server for exact epoch micros for the same wall-clock value.
Int64 expected_micros = 0;
client.Select(
"SELECT toUnixTimestamp64Micro(toDateTime64('2024-10-10 11:00:00', 6))",
[&expected_micros](const Block& block) {
expected_micros = block[0]->As<ColumnInt64>()->At(0);
});
Comment on lines +172 to +183
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

This Issue398 check computes expected_micros using toDateTime64(...) without an explicit timezone and inserts into a DateTime64(6) column without timezone. That makes the check depend on server timezone and largely tautological (binary insert uses the same server-derived epoch), so it may not detect the original text-vs-binary timezone mismatch. To make this a meaningful guard, use an explicit timezone (e.g., DateTime64(6, 'UTC') and/or toDateTime64(..., 'UTC')) and compare the two inserted rows’ epochs against that expected value.

Copilot uses AI. Check for mistakes.

// Binary insert using the same epoch micros.
Block b;
auto d = std::make_shared<ColumnDateTime64>(6);
d->Append(expected_micros);
b.AppendColumn("dt64", d);
client.Insert("test_datetime64_issue398", b);

// Verify both rows are identical in epoch micros.
std::vector<Int64> rows;
client.Select(
"SELECT toUnixTimestamp64Micro(dt64) FROM test_datetime64_issue398 ORDER BY toUnixTimestamp64Micro(dt64)",
[&rows](const Block& block) {
auto col = block[0]->As<ColumnInt64>();
for (size_t i = 0; i < col->Size(); ++i) {
rows.push_back(col->At(i));
}
});

if (rows.size() != 2 || rows[0] != rows[1]) {
throw std::runtime_error("Issue398 repro: text insert and binary insert are inconsistent.");
}
}

inline void DecimalExample(Client& client) {
Block b;

Expand Down Expand Up @@ -559,6 +596,7 @@ static void RunTests(Client& client) {
CancelableExample(client);
DateExample(client);
DateTime64Example(client);
DateTime64Issue398Check(client);
DecimalExample(client);
Comment on lines 598 to 600
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

PR description states the change is scoped to adding a regression UT in ut/roundtrip_tests.cpp, but this PR also modifies tests/simple/main.cpp (adds a new check and rewrites main()/connection defaults). If those changes are intended, please update the PR description/scope; otherwise consider splitting them into a separate PR to keep the regression-test change focused.

Copilot uses AI. Check for mistakes.
EnumExample(client);
ExecptionExample(client);
Expand All @@ -571,35 +609,20 @@ static void RunTests(Client& client) {
ShowTables(client);
}

int main() {
try {
const auto localHostEndpoint = ClientOptions()
.SetHost( getEnvOrDefault("CLICKHOUSE_HOST", "localhost"))
.SetPort( getEnvOrDefault<size_t>("CLICKHOUSE_PORT", "9000"))
.SetEndpoints({ {"asasdasd", 9000}
,{"localhost"}
,{"noalocalhost", 9000}
})
.SetUser( getEnvOrDefault("CLICKHOUSE_USER", "default"))
.SetPassword( getEnvOrDefault("CLICKHOUSE_PASSWORD", ""))
.SetDefaultDatabase(getEnvOrDefault("CLICKHOUSE_DB", "default"));

{
Client client(ClientOptions(localHostEndpoint)
.SetPingBeforeQuery(true));
RunTests(client);
std::cout << "current endpoint : " << client.GetCurrentEndpoint().value().host << "\n";
}

{
Client client(ClientOptions(localHostEndpoint)
.SetPingBeforeQuery(true)
.SetCompressionMethod(CompressionMethod::LZ4));
RunTests(client);
}
} catch (const std::exception& e) {
std::cerr << "exception : " << e.what() << std::endl;
}
static std::string EnvOrDefault(const char* name, const char* fallback) {
if (const char* v = std::getenv(name)) return v;
return fallback;
}

int main() {
clickhouse::ClientOptions options;
options.SetHost(EnvOrDefault("CLICKHOUSE_HOST", "127.0.0.1"));
options.SetPort(static_cast<uint16_t>(std::stoul(EnvOrDefault("CLICKHOUSE_PORT", "9000"))));
options.SetUser(EnvOrDefault("CLICKHOUSE_USER", "test"));
options.SetPassword(EnvOrDefault("CLICKHOUSE_PASSWORD", "test"));
Comment on lines +621 to +622
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

The default connection credentials were changed to user/password "test"/"test". This diverges from the rest of the repo’s test/bench setup (typically user "default" with empty password) and will fail against a standard local ClickHouse instance unless users set env vars. Recommend keeping the prior defaults (or at least matching the repo-wide convention) and only overriding via CLICKHOUSE_* env vars.

Suggested change
options.SetUser(EnvOrDefault("CLICKHOUSE_USER", "test"));
options.SetPassword(EnvOrDefault("CLICKHOUSE_PASSWORD", "test"));
options.SetUser(EnvOrDefault("CLICKHOUSE_USER", "default"));
options.SetPassword(EnvOrDefault("CLICKHOUSE_PASSWORD", ""));

Copilot uses AI. Check for mistakes.
options.SetDefaultDatabase(EnvOrDefault("CLICKHOUSE_DB", "default"));

clickhouse::Client ch_client(options);
RunTests(ch_client);
return 0;
Comment on lines +617 to 627
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Port parsing uses std::stoul(...) and then static_cast<uint16_t>(...). If CLICKHOUSE_PORT is out of range, this will silently truncate to an unexpected port. Consider validating the parsed value is within [1, 65535] (and surfacing a clear error) before casting.

Suggested change
int main() {
clickhouse::ClientOptions options;
options.SetHost(EnvOrDefault("CLICKHOUSE_HOST", "127.0.0.1"));
options.SetPort(static_cast<uint16_t>(std::stoul(EnvOrDefault("CLICKHOUSE_PORT", "9000"))));
options.SetUser(EnvOrDefault("CLICKHOUSE_USER", "test"));
options.SetPassword(EnvOrDefault("CLICKHOUSE_PASSWORD", "test"));
options.SetDefaultDatabase(EnvOrDefault("CLICKHOUSE_DB", "default"));
clickhouse::Client ch_client(options);
RunTests(ch_client);
return 0;
static uint16_t ParsePortFromEnvOrDefault(const char* name, const char* fallback) {
const std::string port_str = EnvOrDefault(name, fallback);
size_t parsed = 0;
const unsigned long port = std::stoul(port_str, &parsed);
if (parsed != port_str.size()) {
throw std::invalid_argument(std::string(name) + " must be a valid integer port number");
}
if (port < 1 || port > 65535) {
throw std::out_of_range(std::string(name) + " must be in range [1, 65535]");
}
return static_cast<uint16_t>(port);
}
int main() {
try {
clickhouse::ClientOptions options;
options.SetHost(EnvOrDefault("CLICKHOUSE_HOST", "127.0.0.1"));
options.SetPort(ParsePortFromEnvOrDefault("CLICKHOUSE_PORT", "9000"));
options.SetUser(EnvOrDefault("CLICKHOUSE_USER", "test"));
options.SetPassword(EnvOrDefault("CLICKHOUSE_PASSWORD", "test"));
options.SetDefaultDatabase(EnvOrDefault("CLICKHOUSE_DB", "default"));
clickhouse::Client ch_client(options);
RunTests(ch_client);
return 0;
} catch (const std::exception& ex) {
std::cerr << "Configuration error: " << ex.what() << std::endl;
return 1;
}

Copilot uses AI. Check for mistakes.
}
43 changes: 43 additions & 0 deletions ut/roundtrip_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,46 @@ INSTANTIATE_TEST_SUITE_P(
.SetCompressionMethod(CompressionMethod::ZSTD)
.SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(false)
));


TEST(DateTime64, Issue398_TextAndBinarySameEpoch) {
Client client(LocalHostEndpoint);

const char* table = "test_datetime64_issue398";
client.Execute(std::string("DROP TABLE IF EXISTS ") + table);
client.Execute(std::string("CREATE TABLE ") + table + " (dt64 DateTime64(6, 'UTC')) ENGINE = Memory");
Comment on lines +345 to +347
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

This test creates/drops a persistent table with a fixed name. If an assertion fails before the final DROP, the table will be left behind and can make subsequent test runs flaky (and can conflict under parallel test execution). Prefer using a TEMPORARY table (session-scoped) and/or a unique table name (e.g., with a UUID suffix), and ensure cleanup via RAII/SCOPE_EXIT so it runs even on ASSERT failures.

Copilot uses AI. Check for mistakes.

// Text path
client.Execute(std::string("INSERT INTO ") + table + " VALUES ('2024-10-10 11:00:00')");

// Binary path (2024-10-10 11:00:00 UTC in microseconds)
constexpr Int64 kEpochMicros = 1728558000000000LL;
{
Block b;
auto c = std::make_shared<ColumnDateTime64>(6);
c->Append(kEpochMicros);
b.AppendColumn("dt64", c);
client.Insert(table, b);
}

bool got_result = false;
bool same_epoch = false;

client.Select(
std::string("SELECT toUInt8(count() = 2 AND min(v) = max(v)) ")
+ "FROM (SELECT toUnixTimestamp64Micro(dt64) AS v FROM " + table + ")",
[&](const Block& block) {
if (block.GetColumnCount() == 0 || block.GetRowCount() == 0) {
return;
}
auto col = block[0]->As<ColumnUInt8>();
ASSERT_TRUE(col != nullptr);
same_epoch = (col->At(0) != 0);
got_result = true;
});

ASSERT_TRUE(got_result);
EXPECT_TRUE(same_epoch);

client.Execute(std::string("DROP TABLE IF EXISTS ") + table);
}
Loading