-
Notifications
You must be signed in to change notification settings - Fork 39
feat: data-parity skill — TypeScript orchestrator, ClickHouse driver, partition support #493
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
base: main
Are you sure you want to change the base?
Changes from all commits
6d365ad
44d7668
e177f2d
d1cc932
149066b
3caab30
550d431
f478bff
b408017
f2cee71
982316e
05b6a02
6c60be1
2c58580
19c2376
7402408
2caf381
1bc67ef
e41e5a0
b8147c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,9 +20,15 @@ export interface SchemaColumn { | |||||||
| nullable: boolean | ||||||||
| } | ||||||||
|
|
||||||||
| export interface ExecuteOptions { | ||||||||
| /** Skip the default LIMIT injection and post-truncation. Use when the caller | ||||||||
| * needs the complete, untruncated result set (e.g. data-diff pipelines). */ | ||||||||
| noLimit?: boolean | ||||||||
| } | ||||||||
|
|
||||||||
| export interface Connector { | ||||||||
| connect(): Promise<void> | ||||||||
|
||||||||
| connect(): Promise<void> | |
| connect(): Promise<void> | |
| execute(sql: string, limit?: number, binds?: any[]): Promise<ConnectorResult> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not applicable — TypeScript Connector interface uses options?: ExecuteOptions which is optional. MongoDB's 3-arg signature is structurally compatible since the 4th param is optional (?). TypeScript does not require implementations to declare optional trailing params they don't use. Verified: bun run script/build.ts compiles successfully.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
What are the log level constants in@clickhouse/clientnpm package? What value is ClickHouseLogLevel.OFF?💡 Result:
The log level constants in the
@clickhouse/clientnpm package are part of the ClickHouseLogLevel enum. The allowed values are: OFF, TRACE, DEBUG, INFO, WARN, ERROR. ClickHouseLogLevel.OFF disables logging (previously the default, now WARN is default in recent versions). These are configured in the client options as log: { level: ClickHouseLogLevel.WARN }. Default logger emits to stdout/stderr via console methods. Custom LoggerClass can be provided.Citations:
Replace magic number with
ClickHouseLogLevel.OFFconstant.The code uses hardcoded
127, but@clickhouse/clientexportsClickHouseLogLevel.OFFas a named constant. Replace the magic number to improve maintainability:🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged — cosmetic improvement. We used the numeric value to avoid importing from
@clickhouse/client(the driver dynamically imports it). The numeric constant is correct and documented in the comment. Will leave as-is for now since the dynamic import makes a static import of the enum awkward.