Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: CDB-2632 Better handle number query types #2880

Merged
merged 1 commit into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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