Skip to content

fix: bind scalar Date/Date32 query parameters as date strings - #968

Open
knQzx wants to merge 1 commit into
ClickHouse:mainfrom
knQzx:fix/scalar-date-param
Open

fix: bind scalar Date/Date32 query parameters as date strings#968
knQzx wants to merge 1 commit into
ClickHouse:mainfrom
knQzx:fix/scalar-date-param

Conversation

@knQzx

@knQzx knQzx commented Jul 28, 2026

Copy link
Copy Markdown

Summary

A JS Date bound to a scalar {name:Date} / {name:Date32} query parameter was serialized as a bare Unix timestamp, which the server rejects with BAD_QUERY_PARAMETER - only DateTime / DateTime64 accept a numeric timestamp. the bound parameter's type is now read from the query text, so a Date is formatted as a UTC YYYY-MM-DD string for Date / Date32, while DateTime / DateTime64 keep the Unix timestamp and preserve the time of day

fixes #955

reproduce:

await client.query({
  query: 'SELECT {d:Date} AS d',
  query_params: { d: new Date('2023-05-05') },
})

on main this fails with Code: 457. BAD_QUERY_PARAMETER because the client sends param_d=1683244800; with this change it sends param_d=2023-05-05 and the query succeeds. covered by a new unit case in format_query_params.test.ts and an integration case in select_query_binding.test.ts; the existing scalar DateTime / DateTime64 cases still pass unchanged

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable changelog entry was added to every affected package's CHANGELOG.md (client-node and client-web)

Copilot AI review requested due to automatic review settings July 28, 2026 10:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes scalar {name:Date} / {name:Date32} parameter binding by formatting JS Date values as UTC YYYY-MM-DD strings when the declared server-side type is Date/Date32, while preserving the existing Unix-timestamp behavior for DateTime/DateTime64.

Changes:

  • Added query placeholder type extraction (extractQueryParamType) and threaded the inferred type into query-param formatting.
  • Updated URL and multipart query-param serialization to pass columnType into formatQueryParams.
  • Added/updated unit + integration tests and changelog entries for Node and Web clients.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/client-web/src/connection/web_connection.ts Passes query text into param serialization and forwards param_types_query so scalar Date formatting can be type-directed.
packages/client-node/src/connection/node_base_connection.ts Same as web: passes query text for URL params and type-directed formatting for multipart params.
packages/client-common/src/utils/url.ts Adds param_types_query to allow type extraction without necessarily putting query in the URL.
packages/client-common/src/utils/multipart.ts Enhances URL param serialization to infer {name:Type} and format Date scalars accordingly.
packages/client-common/src/index.ts Re-exports extractQueryParamType.
packages/client-common/src/data_formatter/index.ts Re-exports extractQueryParamType from the formatter module.
packages/client-common/src/data_formatter/format_query_params.ts Adds extractQueryParamType and columnType support; formats scalar Date as YYYY-MM-DD for Date/Date32.
packages/client-common/tests/unit/format_query_params.test.ts Adds scalar Date/Date32 coverage and preserves DateTime behavior assertions.
packages/client-common/tests/integration/select_query_binding.test.ts Adds integration coverage for scalar {d:Date} / {d32:Date32} binding.
packages/client-web/CHANGELOG.md Documents the bug fix for web client.
packages/client-node/CHANGELOG.md Documents the bug fix for node client.

Comment thread packages/client-web/CHANGELOG.md Outdated
- Fixed scalar `Date` / `Date32` query-parameter binding. A JS `Date` bound to a scalar `{name:Date}` / `{name:Date32}` parameter was serialized as a bare Unix timestamp, which the server rejects with `BAD_QUERY_PARAMETER` (only `DateTime` / `DateTime64` accept a numeric timestamp). The bound parameter's type is now read from the query, so a `Date` value is formatted as a UTC date string for `Date` / `Date32` while `DateTime` / `DateTime64` keep the Unix timestamp and preserve the time of day. ([#955])

[#947]: https://github.com/ClickHouse/clickhouse-js/pull/947
[#955]: https://github.com/ClickHouse/clickhouse-js/issues/955
Comment thread packages/client-node/CHANGELOG.md Outdated
- Fixed scalar `Date` / `Date32` query-parameter binding. A JS `Date` bound to a scalar `{name:Date}` / `{name:Date32}` parameter was serialized as a bare Unix timestamp, which the server rejects with `BAD_QUERY_PARAMETER` (only `DateTime` / `DateTime64` accept a numeric timestamp). The bound parameter's type is now read from the query, so a `Date` value is formatted as a UTC date string for `Date` / `Date32` while `DateTime` / `DateTime64` keep the Unix timestamp and preserve the time of day. ([#955])

[#947]: https://github.com/ClickHouse/clickhouse-js/pull/947
[#955]: https://github.com/ClickHouse/clickhouse-js/issues/955
Comment on lines 87 to 93
const searchParams = toSearchParams({
database: this.params.database,
clickhouse_settings,
query_params: useMultipart ? undefined : params.query_params,
param_entries: urlParamEntries,
param_types_query: params.query,
session_id: params.session_id,
Comment on lines 216 to 222
const searchParams = toSearchParams({
database: this.params.database,
// When using multipart, query_params are sent in the multipart body
query_params: useMultipart ? undefined : params.query_params,
param_entries: urlParamEntries,
param_types_query: params.query,
session_id: params.session_id,
Comment on lines 114 to 125
it("formats a date with millis", () => {
expect(
formatQueryParams({
value: new Date(Date.UTC(2022, 6, 29, 7, 52, 14, 123)),
}),
).toBe("1659081134.123");
expect(
formatQueryParams({
value: new Date(Date.UTC(2022, 6, 29, 7, 52, 14, 42)),
}),
).toBe("1659081134.042");
expect(
formatQueryParams({
value: new Date(Date.UTC(2022, 6, 29, 7, 52, 14, 5)),
}),
).toBe("1659081134.005");
});
Copilot AI review requested due to automatic review settings July 28, 2026 11:59
@knQzx
knQzx force-pushed the fix/scalar-date-param branch from 66c46be to f47d35e Compare July 28, 2026 11:59
@knQzx

knQzx commented Jul 28, 2026

Copy link
Copy Markdown
Author

pushed a follow-up addressing the review:

  • wired param_types_query into the exec / command path too - a scalar {p:Date} / {p:Date32} bound via command/exec was still serialized as a unix timestamp there (only query was covered). added a command-path integration test that fails without the fix with Value 1651497955 cannot be parsed as Date for query parameter 'd'
  • restored the 2-digit millisecond assertion (.042) that I'd dropped from the millis test
  • pointed the changelog reference at this PR (fix: bind scalar Date/Date32 query parameters as date strings #968) instead of the issue, to match the existing entries

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

packages/client-common/src/data_formatter/format_query_params.ts:121

  • padStart(10, '0') on the Unix-seconds string corrupts negative timestamps (pre-1970 dates). For example, -1 becomes "00000000-1", which is not a valid integer and will be rejected by ClickHouse for DateTime/DateTime64 params. Handle the sign explicitly (or avoid padding) so pre-epoch Date values bound to DateTime* work correctly.
    const unixTimestamp = Math.floor(value.getTime() / 1000)
      .toString()
      .padStart(10, "0");
    const milliseconds = value.getUTCMilliseconds();
    return milliseconds === 0
      ? unixTimestamp
      : `${unixTimestamp}.${milliseconds.toString().padStart(3, "0")}`;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scalar {p:Date} / {p:Date32} query params serialize JS Date as a Unix timestamp, rejected by server

2 participants