Skip to content

Commit

Permalink
bug: CDB-2632 Better handle number query types
Browse files Browse the repository at this point in the history 
Fix issues with float casting and truncation in filters by using numeric
types. Clean up and reuse logic for type determinations.
  • Loading branch information
@dbcfd
dbcfd committed Aug 2, 2023
1 parent a7ac706 commit 0851799
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 29 deletions.
26 changes: 19 additions & 7 deletions packages/core/src/indexing/__tests__/query-filter-converter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,31 @@ describe('Should convert query filters', () => {
const query = createQuery({
type: 'where',
value: {
a: { type: 'number', op: '=', value: 1 },
a: { type: 'integer', op: '=', value: 1 },
},
})

expect(query).toEqual(
`select '${DATA_FIELD}' from 'test' where (((cast(${DATA_FIELD}->>'a' as numeric)=1)))`
)
})
test('that are composed of a single doc filter with float', () => {
const query = createQuery({
type: 'where',
value: {
a: { type: 'number', op: '=', value: 1.2 },
},
})

expect(query).toEqual(
`select '${DATA_FIELD}' from 'test' where (((cast(${DATA_FIELD}->>'a' as int)=1)))`
`select '${DATA_FIELD}' from 'test' where (((cast(${DATA_FIELD}->>'a' as numeric)=1.2)))`
)
})
test('that are composed of a single doc filter with null', () => {
const query = createQuery({
type: 'where',
value: {
a: { type: 'number', op: 'null', value: true },
a: { type: 'integer', op: 'null', value: true },
},
})

Expand Down Expand Up @@ -72,7 +84,7 @@ describe('Should convert query filters', () => {
},
})
expect(query).toEqual(
`select '${DATA_FIELD}' from 'test' where (((cast(${DATA_FIELD}->>'a' as int)=1)) and (cast(stream_content->>'b' as int) in (2,3)))`
`select '${DATA_FIELD}' from 'test' where (((cast(${DATA_FIELD}->>'a' as numeric)=1)) and (cast(stream_content->>'b' as numeric) in (2,3)))`
)
})
test('that are composed of and doc filters', () => {
Expand All @@ -94,7 +106,7 @@ describe('Should convert query filters', () => {
],
})
expect(query).toEqual(
`select '${DATA_FIELD}' from 'test' where ((((cast(${DATA_FIELD}->>'a' as int)=1))) and ((cast(stream_content->>'b' as int) in (2,3))))`
`select '${DATA_FIELD}' from 'test' where ((((cast(${DATA_FIELD}->>'a' as numeric)=1))) and ((cast(stream_content->>'b' as numeric) in (2,3))))`
)
})
test('that are composed of or doc filters', () => {
Expand All @@ -116,7 +128,7 @@ describe('Should convert query filters', () => {
],
})
expect(query).toEqual(
`select '${DATA_FIELD}' from 'test' where ((((cast(${DATA_FIELD}->>'a' as int)=1))) or ((cast(stream_content->>'b' as int) in (2,3))))`
`select '${DATA_FIELD}' from 'test' where ((((cast(${DATA_FIELD}->>'a' as numeric)=1))) or ((cast(stream_content->>'b' as numeric) in (2,3))))`
)
})
test('that are composed of or doc filters negated', () => {
Expand All @@ -141,7 +153,7 @@ describe('Should convert query filters', () => {
},
})
expect(query).toEqual(
`select '${DATA_FIELD}' from 'test' where ((not ((cast(${DATA_FIELD}->>'a' as int)=1))) or ((cast(stream_content->>'b' as int) not in (2,3))))`
`select '${DATA_FIELD}' from 'test' where ((not ((cast(${DATA_FIELD}->>'a' as numeric)=1))) or ((cast(stream_content->>'b' as numeric) not in (2,3))))`
)
})
})
38 changes: 27 additions & 11 deletions packages/core/src/indexing/query-filter-converter.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { Knex } from 'knex'
import { NonBooleanValueFilterType, ObjectFilter, QueryFilters } from './query-filter-parser.js'
import {
getValueType,
NonBooleanValueFilterType,
ObjectFilter,
QueryFilters,
ValueFilterType,
} from './query-filter-parser.js'

export const DATA_FIELD = 'stream_content'

Expand Down Expand Up @@ -52,6 +58,24 @@ function handleQuery(
}
}

function typeAsCast(tpe: ValueFilterType): string {
switch (tpe) {
case 'boolean':
return 'boolean'
default:
return nonBooleanTypeAsCast(tpe)
}
}

function nonBooleanTypeAsCast(tpe: NonBooleanValueFilterType): string {
switch (tpe) {
case 'number':
return 'numeric'
case 'string':
return 'varchar'
}
}

function handleIn<T extends number | string>(
query: DBQuery,
key: string,
Expand All @@ -62,10 +86,7 @@ function handleIn<T extends number | string>(
combinator?: Combinator
): DBQuery {
const arrValue = value.map((v) => v.toString()).join(',')
let cast = 'int'
if (tpe == 'string') {
cast = tpe
}
const cast = nonBooleanTypeAsCast(tpe)
const inner = (bldr) => {
let op = ' in '
if (negated) {
Expand Down Expand Up @@ -134,13 +155,8 @@ function handleWhereQuery(state: ConversionState<ObjectFilter>): ConvertedQueryF
}
default: {
const isFirst = first
const cast = typeAsCast(getValueType(value.value))
const old = where
let cast = 'boolean'
if (typeof value.value == 'number') {
cast = 'int'
} else if (typeof value.value == 'string') {
cast = 'varchar'
}
where = (bldr) => {
const b = old(bldr)
let queryValue = value.value
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/indexing/query-filter-parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import {
ObjectFilter as ApiObjectFilter,
} from '@ceramicnetwork/common'

export type NonBooleanValueFilterType = 'number' | 'string'
export type ValueFilterType = 'boolean' | 'number' | 'string'
export type ValueFilter<T extends boolean | number | string = boolean | number | string> =
| { type: ValueFilterType; op: 'null'; value: boolean }
| { type: ValueFilterType; op: '='; value: T }
| { type: ValueFilterType; op: '!='; value: T }

export type NonBooleanValueFilterType = 'number' | 'string'
export type NonBooleanValueFilter<T extends number | string = number | string> =
| { type: NonBooleanValueFilterType; op: 'in'; value: Array<T> }
| { type: NonBooleanValueFilterType; op: 'nin'; value: Array<T> }
Expand All @@ -34,21 +34,21 @@ export type QueryFilters =
| { type: 'or'; value: Array<QueryFilters> }
| { type: 'not'; value: QueryFilters }

function getNonBooleanValueType<T extends number | string>(value: T): NonBooleanValueFilterType {
export function getNonBooleanValueType<T extends number | string>(
value: T
): NonBooleanValueFilterType {
if (typeof value == 'number') {
return 'number'
} else {
return 'string'
}
}

function getValueType<T extends boolean | number | string>(value: T): ValueFilterType {
export function getValueType<T extends boolean | number | string>(value: T): ValueFilterType {
if (typeof value == 'boolean') {
return 'boolean'
} else if (typeof value == 'number') {
return 'number'
} else {
return 'string'
return getNonBooleanValueType(value)
}
}
export function parseObjectFilter(filter: ApiObjectFilter): ObjectFilter {
Expand Down
51 changes: 46 additions & 5 deletions packages/stream-tests/src/__tests__/basic-indexing.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import pgTeardown from '@databases/pg-test/jest/globalTeardown'
import knex, { Knex } from 'knex'
import { INDEXED_MODEL_CONFIG_TABLE_NAME } from '@ceramicnetwork/core'

const CONTENT0 = { myData: 0, myArray: [0] }
const CONTENT0 = { myData: 0, myArray: [0], myFloat: 0.5 }
const CONTENT1 = { myData: 1, myArray: [1], myString: 'a' }
const CONTENT2 = { myData: 2, myArray: [2] }
const CONTENT2 = { myData: 2, myArray: [2], myFloat: 1.0 }
const CONTENT3 = { myData: 3, myArray: [3], myString: 'b' }
const CONTENT4 = { myData: 4, myArray: [4] }
const CONTENT4 = { myData: 4, myArray: [4], myFloat: 1.5 }
const CONTENT5 = { myData: 5, myArray: [5], myString: 'c' }

const MODEL_DEFINITION: ModelDefinition = {
Expand All @@ -44,20 +44,23 @@ const MODEL_DEFINITION: ModelDefinition = {
myArray: {
type: 'array',
items: {
type: 'number',
type: 'integer',
},
},
myString: {
type: 'string',
},
myFloat: {
type: 'number',
},
},
required: ['myData', 'myArray'],
},
}

// The model above will always result in this StreamID when created with the fixed did:key
// controller used by the test.
const MODEL_STREAM_ID = 'kjzl6hvfrbw6c7skfiapcx0ou10qtf0air192h02jtywajoh1djg38ourhi6v3a'
const MODEL_STREAM_ID = 'kjzl6hvfrbw6c5y9s12q66j1mwhmbsp7rhyncq9vigo8nlkf52gn4zg5a8uvbtc'

// StreamID for a model that isn't indexed by the node
const UNINDEXED_MODEL_STREAM_ID = StreamID.fromString(
Expand Down Expand Up @@ -519,6 +522,44 @@ describe.each(envs)('Basic end-to-end indexing query test for $dbEngine', (env)
expect(results.length).toEqual(1)
expect(JSON.stringify(results[0].content)).toEqual(JSON.stringify(doc3.content))
})
test('Can query a single document by float', async () => {
const doc1 = await ModelInstanceDocument.create(ceramic, CONTENT0, midMetadata)
const doc2 = await ModelInstanceDocument.create(ceramic, CONTENT2, midMetadata)
const doc3 = await ModelInstanceDocument.create(ceramic, CONTENT3, midMetadata)
const doc4 = await ModelInstanceDocument.create(ceramic, CONTENT4, midMetadata)
const doc5 = await ModelInstanceDocument.create(ceramic, CONTENT5, midMetadata)

const resultObj0 = await ceramic.index.query({
model: model.id,
last: 5,
queryFilters: {
where: { myFloat: { greaterThan: 1.2 } },
},
})

const results = extractDocuments(ceramic, resultObj0)
expect(results.length).toEqual(1)
expect(JSON.stringify(results[0].content)).toEqual(JSON.stringify(doc4.content))
})
test('Can query a single document by float that gets truncated', async () => {
const doc1 = await ModelInstanceDocument.create(ceramic, CONTENT0, midMetadata)
const doc2 = await ModelInstanceDocument.create(ceramic, CONTENT2, midMetadata)
const doc3 = await ModelInstanceDocument.create(ceramic, CONTENT3, midMetadata)
const doc4 = await ModelInstanceDocument.create(ceramic, CONTENT4, midMetadata)
const doc5 = await ModelInstanceDocument.create(ceramic, CONTENT5, midMetadata)

const resultObj0 = await ceramic.index.query({
model: model.id,
last: 5,
queryFilters: {
where: { myFloat: { greaterThan: 1.0 } },
},
})

const results = extractDocuments(ceramic, resultObj0)
expect(results.length).toEqual(1)
expect(JSON.stringify(results[0].content)).toEqual(JSON.stringify(doc4.content))
})
})

describe('Queries with filters on relations', () => {
Expand Down

0 comments on commit 0851799

Please sign in to comment.