{182577691} fix: make tonumber accept comdb2 number#5760
{182577691} fix: make tonumber accept comdb2 number#5760emelialei88 merged 3 commits intobloomberg:mainfrom
Conversation
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: Success ✓.
The first 10 failing tests are:
disttxn
consumer_non_atomic_default_consumer_generated
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: Success ✓.
The first 10 failing tests are:
sc_truncate_multiddl_generated
autoanalyze
consumer_non_atomic_default_consumer_generated
remsql_locks_rte_connect_generated
remsql_locks
reco-ddlk-sql
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: 0/0 tests failed ⚠.
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: Success ✓.
The first 10 failing tests are:
nogensc
reco-ddlk-sql
consumer_non_atomic_default_consumer_generated
akshatsikarwar
force-pushed
the
fix/tonumber
branch
from
6f43816 to
1113bc1
Compare
|
This was failing robomark build: I rebased the branch and submitted for robom. |
| @@ -0,0 +1,88 @@ | |||
| DROP TABLE IF EXISTS t19 | |||
There was a problem hiding this comment.
Add test for decimal type's exclusion
| if (bb->is_null) return NULL; | ||
| switch (bb->dbtype) { | ||
| case DBTYPES_INTEGER: | ||
| setnvalue(n, (lua_Number)(((lua_int_t *)bb)->val)); |
There was a problem hiding this comment.
add test for number close to max longlong?
| create procedure tonumber_arith_null_int version 'sptest' { | ||
| local function main() | ||
| local row = db:exec("select i from t19 where i is null"):fetch() | ||
| db:emit(row.i + 10) |
There was a problem hiding this comment.
add tests where lua_number comes before comdb2's? like 10 + row.i
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
sc_truncate
queuedb_rollover
sp_snapshot_generated
consumer_non_atomic_default_consumer_generated
bulkimport
reco-ddlk-sql
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated
sc_transactional_rowlocks_generated
reco-ddlk-sql
Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
emelialei88
force-pushed
the
fix/tonumber
branch
from
1113bc1 to
a141b32
Compare
4 participants
1. Make
luaV_tonumberhandle comdb2 integer and real typesExtended
luaV_tonumberto convert comdb2intandrealuserdata to Lua numbers, enabling arithmetic between comdb2 types and Lua numbers without going through metamethods.2. Remove broken userdata handling from
OP_MOVEandOP_LOADKBoth opcodes had runtime checks attempting to detect typed comdb2 variables and route through the
__typemetamethod. These checks were buggy (check lopcodes.c for more information):OP_LOADKisiABxformat (B and C are combined into one 18-bit Bx field), but the code calledRKB(i)which reads the upper 9 bits of Bx as a register index — pointing to an unrelated register.OP_MOVEreadRKC(i)where C is unused, always resolving to R(0).Both could accidentally trigger the typed assignment path when an unrelated register held a comdb2 userdata.
These checks are also unnecessary: the compiler emits
OP_CASTfor typed assignment (:=) and plainOP_MOVE/OP_LOADKfor regular assignment (=). Per documentation, plain=is intended to lose type. No tests rely on this runtime behavior.3. Restore
lua_to_client_typecheck orderComdb2 userdata must be identified before
lua_isnumber/lua_isstring, sinceluaV_tonumbernow succeeds on comdb2 int/real and would misclassify them asSQLITE_FLOAT.