Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

TableDefinition::qualified_name() quotes conditionally, so a reserved-word table name breaks the inserters and to_drop_sql

Aperta
#265 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
68/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
rust, sql

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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 shape to_drop_sql emits — 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 COPY with "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:481 and :713 — sync Inserter
  • hyperdb-api/src/async_inserter.rs:252AsyncInserter
  • hyperdb-api/src/arrow_inserter.rs:189ArrowInserter
  • hyperdb-api/src/async_arrow_inserter.rs:115 and :460AsyncArrowInserter / AsyncArrowInserterOwned
  • hyperdb-api-node/src/inserter.rs:245 — Node bindings
  • hyperdb-api/src/table_definition.rs:987to_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.

Lingua principale
Rust
Stelle
2
Fork
2
Merge medio
12h 2m
PR unite (30g)
60

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di tableau/hyper-api-rust

Tutte le issue di tableau/hyper-api-rust

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.