Skip to content

Commit

Permalink
fix: expressions with not on HANA (#925)
Browse files Browse the repository at this point in the history 
Co-authored-by: Bob den Os <bob.den.os@sap.com>
  • Loading branch information
@vkozyura @BobdenOs
vkozyura and BobdenOs authored Dec 12, 2024
1 parent b519696 commit e67a31b
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 38 deletions.
81 changes: 51 additions & 30 deletions hana/lib/HANAService.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,36 +819,37 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
}

where(xpr) {
xpr = { xpr }
xpr = { xpr, top: true }
const suffix = this.is_comparator(xpr)
return `${this.xpr(xpr)}${suffix ? '' : ` = ${this.val({ val: true })}`}`
return `${this.xpr(xpr, true)}${suffix ? '' : ` = ${this.val({ val: true })}`}`
}

having(xpr) {
return this.where(xpr)
}

xpr(_xpr, caseSuffix = '') {
const { xpr, _internal } = _xpr
xpr(_xpr, iscompare) {
const { xpr, top, _internal } = _xpr
// Maps the compare operators to what to return when both sides are null
const compareOperators = {
const compareTranslations = {
'==': true,
'!=': false,

// These operators are not allowed in column expressions
/* REVISIT: Only adjust these operators when inside the column expression
}
const expressionTranslations = { // These operators are not allowed in column expressions
'==': true,
'!=': false,
'=': null,
'>': null,
'<': null,
'<>': null,
'>=': null,
'<=': null,
'!<': null,
'!>': null,
*/
}

let endWithCompare = false
if (!_internal) {
let curIsCompare = iscompare
for (let i = 0; i < xpr.length; i++) {
let x = xpr[i]
if (typeof x === 'string') {
Expand Down Expand Up @@ -879,11 +880,10 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
// const effective = x === '=' && xpr[i + 1]?.val === null ? '==' : x
// HANA does not support comparators in all clauses (e.g. SELECT 1>0 FROM DUMMY)
// HANA does not have an 'IS' or 'IS NOT' operator
if (x in compareOperators) {
endWithCompare = true
if (curIsCompare ? x in compareTranslations : x in expressionTranslations) {
const left = xpr[i - 1]
const right = xpr[i + 1]
const ifNull = compareOperators[x]
const ifNull = expressionTranslations[x]
x = x === '==' ? '=' : x

const compare = [left, x, right]
Expand Down Expand Up @@ -915,31 +915,35 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE

xpr[i - 1] = ''
xpr[i] = expression
xpr[i + 1] = ' = TRUE'
xpr[i + 1] = curIsCompare ? ' = TRUE' : ''
} else {
const up = x.toUpperCase()
if (up in caseOperators) {
curIsCompare = caseOperators[up]
}
}
}
}
}

const sql = []
let curIsCompare = iscompare
let curIsTop = top
for (let i = 0; i < xpr.length; ++i) {
const x = xpr[i]
if (typeof x === 'string') {
const up = x.toUpperCase()
if (up in logicOperators) {
// Force current expression to end with a comparison
endWithCompare = true
}
if (endWithCompare && (up in caseOperators || up === ')')) {
endWithCompare = false
if (up in caseOperators) {
curIsCompare = caseOperators[up]
curIsTop = caseOperators[up]
}
sql.push(this.operator(x, i, xpr))
} else if (x.xpr) sql.push(`(${this.xpr(x, caseSuffix)})`)
sql.push(this.operator(x, i, xpr, curIsTop))
} else if (x.xpr) sql.push(`(${this.xpr(x, curIsCompare)})`)
// default
else sql.push(this.expr(x))
}

if (endWithCompare) {
if (iscompare) {
const suffix = this.operator('OR', xpr.length, xpr).slice(0, -3)
if (suffix) {
sql.push(suffix)
Expand All @@ -949,12 +953,13 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
return `${sql.join(' ')}`
}

operator(x, i, xpr) {
operator(x, i, xpr, top) {
const up = x.toUpperCase()
// Add "= TRUE" before THEN in case statements
if (
up in logicOperators &&
!this.is_comparator({ xpr }, i - 1)
logicOperators[up] &&
this.is_comparator({ xpr, top }, i - 1) === false
) {
return ` = ${this.val({ val: true })} ${x}`
}
Expand All @@ -973,7 +978,7 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
* @param {} xpr
* @returns
*/
is_comparator({ xpr }, start) {
is_comparator({ xpr, top }, start) {
const local = start != null
for (let i = start ?? xpr.length; i > -1; i--) {
const cur = xpr[i]
Expand All @@ -985,12 +990,25 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
if (up in logicOperators) {
// ensure AND is not part of BETWEEN
if (up === 'AND' && xpr[i - 2]?.toUpperCase?.() in { 'BETWEEN': 1, 'NOT BETWEEN': 1 }) return true
// ensure NOT is not part of a compare operator
if (up === 'NOT' && xpr[i - 1]?.toUpperCase?.() in compareOperators) return true
return !local
}
// When a compare operator is found the expression is a comparison
if (up in compareOperators) return true
// When there is an END of a case statement walk around it to keep checking
if (up === 'END') {
let casedepth = 0
for (; i > -1; i--) {
const up = xpr[i]?.toUpperCase?.()
if (up === 'END') casedepth++
if (up === 'CASE') casedepth--
if (casedepth === 0) break
}
if (casedepth > 0) return false
}
// When a case operator is found it is the start of the expression
if (up in caseOperators) break
if (up in caseOperators) return false
continue
}
if (cur.func?.toUpperCase() === 'CONTAINS' && cur.args?.length > 2) return true
Expand All @@ -1000,7 +1018,7 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
if (nested) return true
}
}
return false
return top ? false : 0
}

list(list) {
Expand Down Expand Up @@ -1357,16 +1375,19 @@ function _not_unique(err, code) {
const is_regexp = x => x?.constructor?.name === 'RegExp' // NOTE: x instanceof RegExp doesn't work in repl
const ObjectKeys = o => (o && [...ObjectKeys(o.__proto__), ...Object.keys(o)]) || []

// All case key words and whether they start an comparison or an expression
const caseOperators = {
'CASE': 1,
'WHEN': 1,
'THEN': 1,
'ELSE': 1,
'THEN': 0,
'ELSE': 0,
}
// All logic operators and whether they have a left hand comparison
const logicOperators = {
'THEN': 1,
'AND': 1,
'OR': 1,
'NOT': 0,
}
const compareOperators = {
'=': 1,
Expand Down
146 changes: 138 additions & 8 deletions test/compliance/SELECT.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,15 @@ describe('SELECT', () => {
await expect(cds.run(cqn)).rejected
})

test.skip('select xpr', async () => {
test('select xpr', async () => {
// REVISIT: Make HANAService ANSI SQL compliant by wrapping compare expressions into case statements for columns
const { string } = cds.entities('basic.projection')
const cqn = CQL`SELECT (${'yes'} = string) as xpr : cds.Boolean FROM ${string}`
const cqn = CQL`SELECT (${'yes'} = string) as xpr : cds.Boolean FROM ${string} order by string`
const res = await cds.run(cqn)
assert.strictEqual(res.length, 3, 'Ensure that all rows are coming back')
assert.equal(res[0].xpr, true)
assert.equal(res[0].xpr, null)
assert.equal(res[1].xpr, false)
assert.equal(res[2].xpr, false)
assert.equal(res[2].xpr, true)
})

test('select calculation', async () => {
Expand Down Expand Up @@ -433,7 +433,7 @@ describe('SELECT', () => {
// search tests don't check results as the search behavior is undefined
test('search one column', async () => {
const { string } = cds.entities('basic.literals')
const cqn = SELECT.from(string).where([{func: 'search', args: [{list: [{ref: ['string']}]}, {val: 'yes'}]}])
const cqn = SELECT.from(string).where([{ func: 'search', args: [{ list: [{ ref: ['string'] }] }, { val: 'yes' }] }])
await cds.run(cqn)
})

Expand Down Expand Up @@ -468,6 +468,136 @@ describe('SELECT', () => {
const res = await cds.run(cqn)
assert.strictEqual(res.length, 3, `Ensure that only matches comeback`)
})

test('deep nested not', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not startswith(string,${'n'})`] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('deep nested boolean function w/o operator', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`startswith(string,${'n'})`] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'no')
})

test('deep nested not + and', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not startswith(string,${'n'}) and not startswith(string,${'n'})`] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('multiple levels of not negations of expressions', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not startswith(string,${'n'})`] }] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('multiple not in a single deep nested expression', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not not not startswith(string,${'n'})`] }}`
await cds.tx(async tx => {
let res
try {
res = await tx.run(query)
} catch (err) {
if (tx.dbc.server.major < 4) return // not not is not supported by older HANA versions
throw err
}
assert.strictEqual(res[0].string, 'yes')
})
})

test('multiple levels of not negations of expression with not + and', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not startswith(string,${'n'}) and not startswith(string,${'n'})`] }] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('multiple levels of not negations of expression with multiple not in a single expression', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not not not startswith(string,${'n'}) and not not not startswith(string,${'n'})`] }] }}`
await cds.tx(async tx => {
let res
try {
res = await tx.run(query)
} catch (err) {
if (tx.dbc.server.major < 4) return // not not is not supported by older HANA versions
throw err
}
assert.strictEqual(res[0].string, 'yes')
})
})

test('deep nested not before xpr with CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', CXL`string = 'no' ? true : false`] }] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('deep nested multiple not before xpr with CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', 'not', 'not', CXL`string = 'no' ? true : false`] }] }}`
await cds.tx(async tx => {
let res
try {
res = await tx.run(query)
} catch (err) {
if (tx.dbc.server.major < 4) return // not not is not supported by older HANA versions
throw err
}
assert.strictEqual(res[0].string, 'yes')
})
})

test('deep nested not before CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr] }] }}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('deep nested multiple not before CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', 'not', 'not', ...(CXL`string = 'no' ? true : false`).xpr] }] }}`
await cds.tx(async tx => {
let res
try {
res = await tx.run(query)
} catch (err) {
if (tx.dbc.server.major < 4) return // not not is not supported by older HANA versions
throw err
}
assert.strictEqual(res[0].string, 'yes')
})
})

test('not before CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr]}}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test.skip('and beetwen CASE statements', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [...(CXL`string = 'no' ? true : false`).xpr, 'and', ...(CXL`string = 'no' ? true : false`).xpr]}}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'no')
})

test('and beetwen CASE statements with not', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr, 'and', 'not', ...(CXL`string = 'no' ? true : false`).xpr]}}`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})
})

describe('groupby', () => {
Expand Down Expand Up @@ -575,7 +705,7 @@ describe('SELECT', () => {
const desc = SELECT.from(string).columns('string').orderBy('string desc')
const mixedAsc = SELECT.from(string).columns('string').orderBy('string aSC')
const asc = SELECT.from(string).columns('string').orderBy('string asc')

expect(await cds.run(mixedDesc)).to.eql(await cds.run(desc))
expect(await cds.run(mixedAsc)).to.eql(await cds.run(asc))
})
Expand Down Expand Up @@ -1006,7 +1136,7 @@ describe('SELECT', () => {
unified.scalar = [
// TODO: investigate search issue for nvarchar columns
...unified.ref.filter(ref => cds.builtin.types[ref.element?.type] === cds.builtin.types.LargeString).map(ref => {
return unified.string.map(val => ({ func: 'search', args: [{list:[ref]}, val] }))
return unified.string.map(val => ({ func: 'search', args: [{ list: [ref] }, val] }))
}).flat(),
// ...unified.string.map(val => ({ func: 'search', args: [{ list: unified.ref.filter(stringRefs) }, val] })),
...unified.ref.filter(stringRefs).filter(noBooleanRefs).map(X => {
Expand Down Expand Up @@ -1236,7 +1366,7 @@ describe('SELECT', () => {
for (const comp of unified.comparators) {
yield { xpr: ['CASE', 'WHEN', comp, 'THEN', { val: true }, 'ELSE', { val: false }, 'END'], as: 'xpr' }
if (!minimal || unified.comparators[0] === comp) {
yield { xpr: ['CASE', 'WHEN', { xpr: ['NOT', ...comp.xpr] }, 'THEN', { val: true }, 'ELSE', { val: false }, 'END'], as: 'xpr' }
yield { xpr: ['CASE', 'WHEN', { xpr: ['NOT', comp] }, 'THEN', { val: true }, 'ELSE', { val: false }, 'END'], as: 'xpr' }
// for (const comp2 of unified.comparators) {
yield { xpr: ['CASE', 'WHEN', comp, 'AND', comp, 'THEN', { val: true }, 'ELSE', { val: false }, 'END'], as: 'xpr' }
yield { xpr: ['CASE', 'WHEN', comp, 'OR', comp, 'THEN', { val: true }, 'ELSE', { val: false }, 'END'], as: 'xpr' }
Expand Down

0 comments on commit e67a31b

Please sign in to comment.