Skip to content

Commit 0b7a47f

Browse files
committed
fix: safely widen numeric list bindings
1 parent 322faca commit 0b7a47f

5 files changed

Lines changed: 448 additions & 12 deletions

File tree

ModuleConfig.cfc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ component {
1818
"convertEmptyStringsToNull": true,
1919
"shouldWrapValues": true,
2020
"validateQueryParamStructKeys": true,
21+
"throwOnUnsafeNumericInference": false,
2122
"integerSQLType": "INTEGER",
2223
"bigIntegerSQLType": "BIGINT",
2324
"decimalSQLType": "DECIMAL",
@@ -55,6 +56,7 @@ component {
5556
.to( "qb.models.Query.QueryUtils" )
5657
.initArg( name = "convertEmptyStringsToNull", value = settings.convertEmptyStringsToNull )
5758
.initArg( name = "validateQueryParamStructKeys", value = settings.validateQueryParamStructKeys )
59+
.initArg( name = "throwOnUnsafeNumericInference", value = settings.throwOnUnsafeNumericInference )
5860
.initArg( name = "integerSQLType", value = settings.integerSQLType )
5961
.initArg( name = "bigIntegerSQLType", value = settings.bigIntegerSQLType )
6062
.initArg( name = "decimalSQLType", value = settings.decimalSQLType );

README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,23 @@ Using qb, you can:
2020

2121
Installation is easy through [CommandBox](https://www.ortussolutions.com/products/commandbox) and [ForgeBox](https://www.coldbox.org/forgebox). Simply type `box install qb` to get started.
2222

23+
## Numeric list inference
24+
25+
qb combines numeric array members using a common SQL type that covers their declared ranges. For example, `[ 1, 3000000000 ]` uses `BIGINT`, and integers mixed with fractional values use `DECIMAL`. Explicit member types such as `TINYINT`, `SMALLINT`, `REAL`, `FLOAT`, and `DOUBLE` also participate in inference.
26+
27+
When there is no portable numeric promotion without potential precision loss, qb falls back to `VARCHAR`. Examples include `BIGINT` mixed with `DOUBLE`, and `DECIMAL` mixed with `FLOAT`. This preserves the binding representation; the database can still apply its own conversion when executing the query.
28+
29+
We recommend enabling `throwOnUnsafeNumericInference` in development to catch these combinations early:
30+
31+
```cfc
32+
// config/ColdBox.cfc, in your development environment configuration
33+
moduleSettings.qb.throwOnUnsafeNumericInference = true;
34+
```
35+
36+
The setting defaults to `false`. When enabled, unsafe numeric array inference throws `QBUnsafeNumericInference` with the conflicting SQL types. Safe promotions and ordinary mixed text arrays retain their normal behavior. For standalone usage, pass `throwOnUnsafeNumericInference = true` to the `QueryUtils` constructor.
37+
38+
An explicit `cfsqltype` or `sqltype` on the outer binding always takes precedence; this setting does not validate caller-selected conversions or database column precision and scale.
39+
2340
## Code Samples
2441

2542
Compare these two examples:

models/Query/QueryUtils.cfc

Lines changed: 96 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ component singleton displayname="QueryUtils" accessors="true" {
3333
*/
3434
property name="decimalSQLType" default="DECIMAL";
3535

36+
/**
37+
* Throw when numeric array types cannot be combined without potential precision loss.
38+
* Recommended in development; otherwise inference falls back to VARCHAR.
39+
*/
40+
property name="throwOnUnsafeNumericInference" default="false";
41+
3642
variables.numericValueTypes = {
3743
"AtomicInteger": true,
3844
"AtomicLong": true,
@@ -85,12 +91,14 @@ component singleton displayname="QueryUtils" accessors="true" {
8591
string integerSqlType = "INTEGER",
8692
string decimalSqlType = "DECIMAL",
8793
any log,
88-
string bigIntegerSqlType = "BIGINT"
94+
string bigIntegerSqlType = "BIGINT",
95+
boolean throwOnUnsafeNumericInference = false
8996
) {
9097
variables.convertEmptyStringsToNull = arguments.convertEmptyStringsToNull;
9198
variables.validateQueryParamStructKeys = arguments.validateQueryParamStructKeys;
9299
variables.integerSqlType = arguments.integerSqlType;
93100
variables.bigIntegerSqlType = arguments.bigIntegerSqlType;
101+
variables.throwOnUnsafeNumericInference = arguments.throwOnUnsafeNumericInference;
94102
variables.decimalSqlType = arguments.decimalSqlType;
95103
if ( !isNull( arguments.log ) ) {
96104
variables.log = arguments.log;
@@ -561,8 +569,7 @@ component singleton displayname="QueryUtils" accessors="true" {
561569
}
562570

563571
if ( isArray( value ) ) {
564-
var inferredType = "";
565-
var hasInferredType = false;
572+
var inferredTypes = {};
566573
for ( var valueIndex = 1; valueIndex <= arguments.value.len(); valueIndex++ ) {
567574
if ( !arrayIsDefined( arguments.value, valueIndex ) || isNull( arguments.value[ valueIndex ] ) ) {
568575
continue;
@@ -572,14 +579,15 @@ component singleton displayname="QueryUtils" accessors="true" {
572579
continue;
573580
}
574581
var itemType = inferSqlType( item, arguments.grammar );
575-
if ( !hasInferredType ) {
576-
inferredType = itemType;
577-
hasInferredType = true;
578-
} else if ( itemType != inferredType ) {
579-
return "VARCHAR";
580-
}
582+
inferredTypes[ itemType ] = true;
583+
}
584+
if ( inferredTypes.isEmpty() ) {
585+
return "VARCHAR";
581586
}
582-
return hasInferredType ? inferredType : "VARCHAR";
587+
if ( inferredTypes.count() == 1 ) {
588+
return inferredTypes.keyArray()[ 1 ];
589+
}
590+
return combineNumericSqlTypes( inferredTypes );
583591
}
584592

585593
if ( isStruct( value ) ) {
@@ -938,6 +946,60 @@ component singleton displayname="QueryUtils" accessors="true" {
938946
return isSimpleValue( arguments.value ) && variables.numericValueTypes.keyExists( type );
939947
}
940948

949+
/**
950+
* Combine declared numeric ranges, rather than narrowing them to the current values.
951+
* Exact decimals and approximate types have no portable, lossless common type.
952+
*/
953+
private string function combineNumericSqlTypes( required struct types ) {
954+
var integerTypes = "BIT,TINYINT,SMALLINT,INTEGER,BIGINT";
955+
var numericTypes = integerTypes & ",MONEY4,MONEY,DECIMAL,NUMERIC,REAL,FLOAT,DOUBLE";
956+
var integerRank = 0;
957+
for ( var sqlType in arguments.types ) {
958+
if ( !listFindNoCase( numericTypes, sqlType ) ) {
959+
return "VARCHAR";
960+
}
961+
integerRank = max( integerRank, listFindNoCase( integerTypes, sqlType ) );
962+
}
963+
964+
var hasDecimal = arguments.types.keyExists( "DECIMAL" ) || arguments.types.keyExists( "NUMERIC" );
965+
var hasMoney = arguments.types.keyExists( "MONEY" ) || arguments.types.keyExists( "MONEY4" );
966+
var hasApproximate = arguments.types.keyExists( "REAL" ) || arguments.types.keyExists( "FLOAT" ) || arguments.types.keyExists( "DOUBLE" );
967+
if ( hasApproximate ) {
968+
if ( hasDecimal || hasMoney || integerRank == 5 ) {
969+
if ( variables.throwOnUnsafeNumericInference ) {
970+
throw(
971+
type = "QBUnsafeNumericInference",
972+
message = "Cannot infer a common numeric SQL type without potential precision loss.",
973+
detail = "Numeric types: [#arguments.types
974+
.keyArray()
975+
.sort( "textnocase" )
976+
.toList( ", " )#]. Specify a SQL type on the outer binding, or disable throwOnUnsafeNumericInference to fall back to VARCHAR."
977+
);
978+
}
979+
return "VARCHAR";
980+
}
981+
if ( arguments.types.keyExists( "DOUBLE" ) ) {
982+
return "DOUBLE";
983+
}
984+
if ( arguments.types.keyExists( "FLOAT" ) ) {
985+
return "FLOAT";
986+
}
987+
// REAL has 24 significant binary digits; use DOUBLE for the full INTEGER range.
988+
return integerRank == 4 ? "DOUBLE" : "REAL";
989+
}
990+
991+
if ( hasDecimal ) {
992+
return arguments.types.keyExists( "DECIMAL" ) ? "DECIMAL" : "NUMERIC";
993+
}
994+
if ( hasMoney ) {
995+
if ( integerRank == 5 ) {
996+
return "DECIMAL";
997+
}
998+
return arguments.types.keyExists( "MONEY" ) || integerRank == 4 ? "MONEY" : "MONEY4";
999+
}
1000+
return listGetAt( integerTypes, integerRank );
1001+
}
1002+
9411003
private string function deriveNumericSqlType( required numeric value ) {
9421004
var isInteger = reFind( "^-?\d+$", arguments.value ) > 0;
9431005
if ( !isInteger ) {
@@ -1139,6 +1201,30 @@ component singleton displayname="QueryUtils" accessors="true" {
11391201
return 0;
11401202
}
11411203

1204+
if ( isArray( arguments.binding.value ) ) {
1205+
var scale = 0;
1206+
for ( var valueIndex = 1; valueIndex <= arguments.binding.value.len(); valueIndex++ ) {
1207+
if (
1208+
!arrayIsDefined( arguments.binding.value, valueIndex ) || isNull(
1209+
arguments.binding.value[ valueIndex ]
1210+
)
1211+
) {
1212+
continue;
1213+
}
1214+
var item = arguments.binding.value[ valueIndex ];
1215+
var itemBinding = isStruct( item ) ? {
1216+
value: isNull( item.value ) ? javacast( "null", "" ) : item.value,
1217+
null: structKeyExists( item, "null" ) && item.null
1218+
} : { value: item, null: false };
1219+
if ( isStruct( item ) && structKeyExists( item, "scale" ) && !itemBinding.null ) {
1220+
scale = max( scale, item.scale );
1221+
} else {
1222+
scale = max( scale, calculateNumberOfDecimalDigits( itemBinding ) );
1223+
}
1224+
}
1225+
return scale;
1226+
}
1227+
11421228
if ( isInstanceOf( arguments.binding.value, "java.math.BigDecimal" ) ) {
11431229
return max( 0, arguments.binding.value.scale() );
11441230
}

tests/specs/ModuleConfigSpec.cfc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ component extends="testbox.system.BaseSpec" {
1616
moduleConfig.configure();
1717

1818
var settings = moduleConfig.$getProperty( "settings", "variables" );
19+
expect( settings.throwOnUnsafeNumericInference ).toBeFalse();
1920
expect( settings.integerSQLType ).toBe( "INTEGER" );
2021
expect( settings.bigIntegerSQLType ).toBe( "BIGINT" );
2122
expect( settings.decimalSQLType ).toBe( "DECIMAL" );

0 commit comments

Comments
 (0)