From 0851799bc16a589acd8f3c324316dc666585460c Mon Sep 17 00:00:00 2001 From: Danny Browning Date: Wed, 2 Aug 2023 15:46:37 -0600 Subject: [PATCH] bug: CDB-2632 Better handle number query types Fix issues with float casting and truncation in filters by using numeric types. Clean up and reuse logic for type determinations. --- .../__tests__/query-filter-converter.test.ts | 26 +++++++--- .../src/indexing/query-filter-converter.ts | 38 ++++++++++---- .../core/src/indexing/query-filter-parser.ts | 12 ++--- .../src/__tests__/basic-indexing.test.ts | 51 +++++++++++++++++-- 4 files changed, 98 insertions(+), 29 deletions(-) diff --git a/packages/core/src/indexing/__tests__/query-filter-converter.test.ts b/packages/core/src/indexing/__tests__/query-filter-converter.test.ts index 927db5e4e8..d3904725c2 100644 --- a/packages/core/src/indexing/__tests__/query-filter-converter.test.ts +++ b/packages/core/src/indexing/__tests__/query-filter-converter.test.ts @@ -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 }, }, }) @@ -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', () => { @@ -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', () => { @@ -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', () => { @@ -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))))` ) }) }) diff --git a/packages/core/src/indexing/query-filter-converter.ts b/packages/core/src/indexing/query-filter-converter.ts index 0861995740..3df853428e 100644 --- a/packages/core/src/indexing/query-filter-converter.ts +++ b/packages/core/src/indexing/query-filter-converter.ts @@ -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' @@ -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( query: DBQuery, key: string, @@ -62,10 +86,7 @@ function handleIn( 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) { @@ -134,13 +155,8 @@ function handleWhereQuery(state: ConversionState): 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 diff --git a/packages/core/src/indexing/query-filter-parser.ts b/packages/core/src/indexing/query-filter-parser.ts index 202dd03d16..4d49ce6b4c 100644 --- a/packages/core/src/indexing/query-filter-parser.ts +++ b/packages/core/src/indexing/query-filter-parser.ts @@ -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 = | { type: ValueFilterType; op: 'null'; value: boolean } | { type: ValueFilterType; op: '='; value: T } | { type: ValueFilterType; op: '!='; value: T } -export type NonBooleanValueFilterType = 'number' | 'string' export type NonBooleanValueFilter = | { type: NonBooleanValueFilterType; op: 'in'; value: Array } | { type: NonBooleanValueFilterType; op: 'nin'; value: Array } @@ -34,7 +34,9 @@ export type QueryFilters = | { type: 'or'; value: Array } | { type: 'not'; value: QueryFilters } -function getNonBooleanValueType(value: T): NonBooleanValueFilterType { +export function getNonBooleanValueType( + value: T +): NonBooleanValueFilterType { if (typeof value == 'number') { return 'number' } else { @@ -42,13 +44,11 @@ function getNonBooleanValueType(value: T): NonBoolean } } -function getValueType(value: T): ValueFilterType { +export function getValueType(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 { diff --git a/packages/stream-tests/src/__tests__/basic-indexing.test.ts b/packages/stream-tests/src/__tests__/basic-indexing.test.ts index 26bdb09c0e..fb8ceda0e1 100644 --- a/packages/stream-tests/src/__tests__/basic-indexing.test.ts +++ b/packages/stream-tests/src/__tests__/basic-indexing.test.ts @@ -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 = { @@ -44,12 +44,15 @@ const MODEL_DEFINITION: ModelDefinition = { myArray: { type: 'array', items: { - type: 'number', + type: 'integer', }, }, myString: { type: 'string', }, + myFloat: { + type: 'number', + }, }, required: ['myData', 'myArray'], }, @@ -57,7 +60,7 @@ const MODEL_DEFINITION: ModelDefinition = { // 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( @@ -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', () => {