TableDefinition::qualified_name() quotes conditionally, so a reserved-word table name breaks the inserters and to_drop_sql
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 68/100
- issue の種類
- バグ
- 明瞭さ
- おおむね明確
- 活発さ
- 活発
調査の方向性
Start in hyperdb-api-core/src/protocol/escape.rs and hyperdb-api/src/table_definition.rs, comparing QuotedIdentifier and quoted_qualified_name() with the existing qualified_name() paths. Trace the inserter entry points listed in the issue and the COPY construction in async_connection.rs and connection.rs. Done means generated DROP and COPY statements safely handle reserved-word table names without changing the display-oriented qualified_name() behavior.
索引モデルが issue の本文から書いたものです。
説明
Summary
TableDefinition::qualified_name() formats identifiers through SqlIdentifier, which omits the quotes when a name already looks like a legal bare identifier. That "looks legal" check has no reserved-word list, so an all-lowercase SQL keyword is emitted unquoted and the resulting statement is a syntax error. The inserters and to_drop_sql build their SQL from qualified_name(), so a table named order (or select, group, user, …) can be created but then cannot be inserted into or dropped through those paths.
The check documents its own gap:
// hyperdb-api-core/src/protocol/escape.rs:110-117
/// Checks if a string is a valid unquoted identifier.
///
/// Valid unquoted identifiers:
/// - Start with a letter (a-z, A-Z) or underscore
/// - Contain only letters, digits (0-9), underscores, and dollar signs
/// - Are not SQL reserved words (this function doesn't check for reserved words)
#[must_use]
pub fn is_valid_unquoted_identifier(s: &str) -> bool {
and SqlIdentifier quotes only when that check fails or the name carries uppercase:
// hyperdb-api-core/src/protocol/escape.rs:58-59
let needs_quoting =
!is_valid_unquoted_identifier(self.0) || self.0.chars().any(char::is_uppercase);
so SqlIdentifier("order") renders as bare order, and that is what qualified_name() returns:
// hyperdb-api/src/table_definition.rs:816-829
pub fn qualified_name(&self) -> String {
match (&self.database, &self.schema) {
// ...
(None, None) => format!("{}", SqlIdentifier(&self.name)),
}
}
Verified against a running engine
Executed against hyperd (the pinned release), not inferred:
CREATE TABLE "order" ("id" INTEGER, "select" TEXT)— accepted. Keyword-named tables are legal, so they do occur in practice.DROP TABLE IF EXISTS order— the exact shapeto_drop_sqlemits — rejected:ERROR: syntax error: got ORDER, expected <identifier> (42601).COPY order ("id", "select") FROM STDIN WITH (FORMAT HYPERBINARY)— the exact shape the inserters build — rejected:ERROR: syntax error: got ORDER, expected one of: <identifier>, '(' (42601).- The same
COPYwith"order"quoted parses and proceeds to await the COPY stream, isolating the quoting as the cause rather than anything else about the statement.
Call sites still routing through qualified_name() on main
The COPY statement is assembled here, interpolating the table name verbatim:
// hyperdb-api-core/src/client/async_connection.rs:691
// (sync twin: hyperdb-api-core/src/client/connection.rs:878)
let query = format!("COPY {table_name}{column_list} FROM STDIN WITH (FORMAT {format})");
and table_name comes from qualified_name() at:
hyperdb-api/src/inserter.rs:481and:713— syncInserterhyperdb-api/src/async_inserter.rs:252—AsyncInserterhyperdb-api/src/arrow_inserter.rs:189—ArrowInserterhyperdb-api/src/async_arrow_inserter.rs:115and:460—AsyncArrowInserter/AsyncArrowInserterOwnedhyperdb-api-node/src/inserter.rs:245— Node bindingshyperdb-api/src/table_definition.rs:987—to_drop_sql
Worth noting the columns in a COPY are already quoted unconditionally (hyperdb-api-core/src/client/async_connection.rs:685), so a keyword column name is fine. Only the table name is exposed.
#258 fixed this class of bug, deliberately without touching qualified_name()
#258 introduced QuotedIdentifier, which quotes unconditionally, for exactly this failure — its doc names select and order:
// hyperdb-api-core/src/protocol/escape.rs:187-195
/// A SQL identifier that is **always** quoted, whatever it contains.
///
/// [`SqlIdentifier`] omits the quotes when a name is already a legal bare
/// identifier, which is fine for display but unsafe for generated DDL:
/// [`is_valid_unquoted_identifier`] deliberately does not know the reserved
/// word list, so an all-lowercase keyword such as `select` or `order` passes
/// the check and is emitted bare, producing a syntax error.
and a private quoted_qualified_name() (hyperdb-api/src/table_definition.rs:792) used by the DDL it generates:
// hyperdb-api/src/table_definition.rs:929 (to_create_sql)
sql.push_str(&self.quoted_qualified_name());
So to_create_sql is already correct on main. to_drop_sql at :987 and every inserter site above still use qualified_name(). #258 left the public qualified_name() alone on purpose — changing it churns public doctests and the examples that print it — so this is the deliberately-deferred remainder, not a regression from that PR.
Impact
Latent, but a hard failure once hit: CREATE TABLE succeeds and then every insert into that table fails with a syntax error naming a SQL keyword rather than the user's table, which reads as a library bug. Most affected users won't have chosen the name — it arrives from a CSV header, a reflected schema, or an upstream system.
Fix direction
Follow #258's shape and route the SQL-generating paths through QuotedIdentifier, rather than loosening SqlIdentifier (which is doing legitimate work for display). to_drop_sql can switch to quoted_qualified_name() outright — it generates SQL and nothing inspects its output format.
The inserters need more care. They take &str table names internally, so they could receive the quoted form at construction, but two things want an audit first: callers that read qualified_name() for display, and any caller passing an already-quoted name in (which would then be double-quoted). Keeping qualified_name() as the display-oriented accessor it has effectively become, and adding a documented SQL-safe counterpart that the SQL paths use, keeps the change additive.
- 主要言語
- Rust
- スター
- 2
- フォーク
- 2
- 平均マージ
- 12時間 2分
- マージ済み PR(30日)
- 60
環境構築
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
tableau/hyper-api-rust のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
tableau/hyper-api-rust#294 ·
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 35/100
tableau/hyper-api-rust#311 ·
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 45/100
tableau/hyper-api-rust#305 ·
メンテナーはふだん 1 日以内に返信
-
Windows Named Pipe: verify DACL denies other users, and measure read-path perf for MCP workloadsオープン
難易度 4/5 3〜5日 初心者へのやさしさ 38/100
tableau/hyper-api-rust#302 ·
メンテナーはふだん 1 日以内に返信
-
難易度 3/5 1〜2日 初心者へのやさしさ 72/100
tableau/hyper-api-rust#300 ·
メンテナーはふだん 1 日以内に返信
tableau/hyper-api-rust の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
arkworks-rs/algebra#1161 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
lbjlaq/Antigravity-Manager#3525 · コメント 2 件 · リアクション 1 件 ·
メンテナーはふだん 1 日以内に返信
-
agent-ready area:platform bug criticality:p3 rust triage:needs-implementation
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
registrystack/registry-stack#1583 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
メンテナーはふだん 1 日以内に返信
-
Docs: "Work with Codex from anywhere" page still claims Windows mobile support is "coming soon"オープンapp documentation remote windows-os
難易度 1/5 1〜3時間 初心者へのやさしさ 88/100
メンテナーはふだん 1 日以内に返信