Skip to content

Commit 74e3329

Browse files
committed
fix(sql): honor declared float width across DDL, DML, and pgwire
REAL/FLOAT4/FLOAT8/DOUBLE PRECISION and their PostgreSQL aliases were rejected as unknown column types, and even where declared, values on untyped write paths (KV, document-schemaless) were stored as decimal text and silently dropped by the pgwire encoder. Add a FloatWidth type mirroring IntWidth (shared declared-type keyword matching factored into declared_type_keyword.rs), coerce float/int literals to their declared representation before storage, and narrow floats on the wire with an overflow-only error, so RowDescription OIDs, catalog introspection, and encoded values all agree with the declared width.
1 parent 98c8a37 commit 74e3329

29 files changed

Lines changed: 2165 additions & 194 deletions

nodedb-sql/src/placeholder_types/slots.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,32 @@
33
//! Per-position inference state: the slot table, its conflict rule, and the
44
//! parsing of a `$N` placeholder body into a position.
55
6-
use nodedb_types::columnar::IntWidth;
6+
use nodedb_types::columnar::{FloatWidth, IntWidth};
77
use sqlparser::ast::{Expr, Value, ValueWithSpan};
88

99
use crate::types::ColumnInfo;
1010
use crate::types_expr::SqlDataType;
1111

1212
/// A parameter type the statement pins down.
1313
///
14-
/// Carries the declared integer width alongside the logical type because the
14+
/// Carries the declared numeric width alongside the logical type because the
1515
/// two are not interchangeable on the wire: a column declared `INT` must be
16-
/// advertised as `int4` (OID 23), not `int8` (OID 20). The client encodes its
17-
/// bind value at exactly the width the `ParameterDescription` names, so
18-
/// collapsing every integer to `int8` here would put a 4-byte column behind an
19-
/// 8-byte promise — the precise failure the catalog's resolved [`IntWidth`]
20-
/// exists to prevent.
16+
/// advertised as `int4` (OID 23), not `int8` (OID 20), and one declared `REAL`
17+
/// as `float4` (OID 700), not `float8` (OID 701). The client encodes its bind
18+
/// value at exactly the width the `ParameterDescription` names, so collapsing
19+
/// every integer to `int8` or every float to `float8` here would put a 4-byte
20+
/// column behind an 8-byte promise — the precise failure the catalog's
21+
/// resolved [`IntWidth`] / [`FloatWidth`] exist to prevent.
2122
#[derive(Debug, Clone, PartialEq)]
2223
pub struct InferredParamType {
2324
/// The logical type this position resolves to.
2425
pub data_type: SqlDataType,
2526
/// Declared width for an integer position, or `None` when the form that
2627
/// resolved it had no catalog column behind it.
2728
pub int_width: Option<IntWidth>,
29+
/// Declared width for a floating-point position, or `None` when the form
30+
/// that resolved it had no catalog column behind it.
31+
pub float_width: Option<FloatWidth>,
2832
}
2933

3034
impl InferredParamType {
@@ -38,6 +42,7 @@ impl InferredParamType {
3842
Self {
3943
data_type,
4044
int_width: None,
45+
float_width: None,
4146
}
4247
}
4348

@@ -47,6 +52,7 @@ impl InferredParamType {
4752
Self {
4853
data_type: column.data_type.clone(),
4954
int_width: column.int_width,
55+
float_width: column.float_width,
5056
}
5157
}
5258
}

nodedb-sql/src/placeholder_types/tests.rs

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! Unit tests for `$N` placeholder type inference.
44
55
use nodedb_types::DatabaseId;
6-
use nodedb_types::columnar::IntWidth;
6+
use nodedb_types::columnar::{FloatWidth, IntWidth};
77

88
use super::infer_placeholder_types;
99
use super::slots::{InferredParamType, parse_placeholder_body};
@@ -24,6 +24,18 @@ fn col(name: &str, data_type: SqlDataType, int_width: Option<IntWidth>) -> Colum
2424
default: None,
2525
raw_type: None,
2626
int_width,
27+
float_width: None,
28+
}
29+
}
30+
31+
/// A float column with a declared width — the float analogue of `col`'s
32+
/// `int_width` argument. Separate because the two families never co-occur on
33+
/// one column, so widening `col` with a fourth argument would make every
34+
/// integer and string call site carry a `None` it can never set.
35+
fn float_col(name: &str, float_width: FloatWidth) -> ColumnInfo {
36+
ColumnInfo {
37+
float_width: Some(float_width),
38+
..col(name, SqlDataType::Float64, None)
2739
}
2840
}
2941

@@ -62,6 +74,8 @@ impl SqlCatalog for TestCatalog {
6274
col("big", SqlDataType::Int64, Some(IntWidth::I64)),
6375
col("label", SqlDataType::String, None),
6476
col("flag", SqlDataType::Bool, None),
77+
float_col("ratio", FloatWidth::F32),
78+
float_col("wide", FloatWidth::F64),
6579
],
6680
)),
6781
// Two relations sharing a column name: the ambiguity lock.
@@ -100,14 +114,25 @@ fn from_sql(data_type: SqlDataType) -> Option<InferredParamType> {
100114
Some(InferredParamType {
101115
data_type,
102116
int_width: None,
117+
float_width: None,
103118
})
104119
}
105120

106-
/// A type taken from a catalog column, declared width included.
121+
/// A type taken from a catalog column, declared integer width included.
107122
fn from_col(data_type: SqlDataType, int_width: Option<IntWidth>) -> Option<InferredParamType> {
108123
Some(InferredParamType {
109124
data_type,
110125
int_width,
126+
float_width: None,
127+
})
128+
}
129+
130+
/// A type taken from a catalog float column, declared float width included.
131+
fn from_float_col(float_width: FloatWidth) -> Option<InferredParamType> {
132+
Some(InferredParamType {
133+
data_type: SqlDataType::Float64,
134+
int_width: None,
135+
float_width: Some(float_width),
111136
})
112137
}
113138

@@ -309,6 +334,21 @@ fn where_comparison_resolves_column_type_and_width() {
309334
);
310335
}
311336

337+
/// The float analogue: a `REAL` column must carry its declared `F32` width so
338+
/// the caller advertises oid 700 and the client encodes four bytes, while a
339+
/// `DOUBLE` column stays `F64` (oid 701).
340+
#[test]
341+
fn where_comparison_resolves_declared_float_width() {
342+
assert_eq!(
343+
infer("SELECT id FROM t WHERE ratio = $1"),
344+
vec![from_float_col(FloatWidth::F32)]
345+
);
346+
assert_eq!(
347+
infer("SELECT id FROM t WHERE wide = $1"),
348+
vec![from_float_col(FloatWidth::F64)]
349+
);
350+
}
351+
312352
/// `$1 = col` is the same form with the operands swapped.
313353
#[test]
314354
fn reversed_operand_order_resolves() {
@@ -418,17 +458,22 @@ fn insert_multi_row_values_map_each_row() {
418458
}
419459

420460
/// No explicit column list: positional against the declared column order,
421-
/// which is only meaningful when the arity matches it exactly.
461+
/// which is only meaningful when the arity matches it exactly. `t` declares
462+
/// seven columns, so seven values are required for the mapping to apply —
463+
/// the two trailing float columns also pin that declared float width rides
464+
/// along the same positional path as integer width.
422465
#[test]
423466
fn insert_without_column_list_maps_against_declared_order() {
424467
assert_eq!(
425-
infer("INSERT INTO t VALUES ($1, $2, $3, $4, $5)"),
468+
infer("INSERT INTO t VALUES ($1, $2, $3, $4, $5, $6, $7)"),
426469
vec![
427470
from_col(SqlDataType::String, None),
428471
from_col(SqlDataType::Int64, Some(IntWidth::I32)),
429472
from_col(SqlDataType::Int64, Some(IntWidth::I64)),
430473
from_col(SqlDataType::String, None),
431474
from_col(SqlDataType::Bool, None),
475+
from_float_col(FloatWidth::F32),
476+
from_float_col(FloatWidth::F64),
432477
]
433478
);
434479
}

0 commit comments

Comments
 (0)