Skip to content

Commit

Permalink
fix: do not apply block range filter on internal tables (#303)
Browse files Browse the repository at this point in the history
This commit also refactors out applyBlockFilter helper into database utils
to avoid repetition.
  • Loading branch information
Sekhmet authored Jun 26, 2024
1 parent b427b15 commit 333ca3f
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 25 deletions.
11 changes: 5 additions & 6 deletions src/graphql/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from 'graphql';
import DataLoader from 'dataloader';
import { ResolverContextInput } from './resolvers';
import { getTableName } from '../utils/database';
import { getTableName, applyBlockFilter } from '../utils/database';

/**
* Creates getLoader function that will return existing, or create a new dataloader
Expand All @@ -25,14 +25,13 @@ export const createGetLoader = (context: ResolverContextInput) => {

if (!loaders[key]) {
loaders[key] = new DataLoader(async ids => {
const tableName = getTableName(name);

let query = context.knex
.select('*')
.from(getTableName(name))
.from(tableName)
.whereIn(field, ids as string[]);
query =
block !== undefined
? query.andWhereRaw('block_range @> int8(??)', [block])
: query.andWhereRaw('upper_inf(block_range)');
query = applyBlockFilter(query, tableName, block);

context.log.debug({ sql: query.toQuery(), ids }, 'executing batched query');

Expand Down
26 changes: 7 additions & 19 deletions src/graphql/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import { Knex } from 'knex';
import { Pool as PgPool } from 'pg';
import { getNonNullType, getDerivedFromDirective } from '../utils/graphql';
import { getTableName } from '../utils/database';
import { getTableName, applyBlockFilter } from '../utils/database';
import { Logger } from '../utils/logger';
import type DataLoader from 'dataloader';

Expand Down Expand Up @@ -71,10 +71,7 @@ export async function queryMulti(
const nestedEntitiesMappings = {} as Record<string, Record<string, string>>;

let query = knex.select(`${tableName}.*`).from(tableName);
query =
args.block !== undefined
? query.andWhereRaw(`${tableName}.block_range @> int8(??)`, [args.block])
: query.andWhereRaw(`upper_inf(${tableName}.block_range)`);
query = applyBlockFilter(query, tableName, args.block);

const handleWhere = (query: Knex.QueryBuilder, prefix: string, where: Record<string, any>) => {
Object.entries(where).map((w: [string, any]) => {
Expand Down Expand Up @@ -132,10 +129,7 @@ export async function queryMulti(
.columns(nestedEntitiesMappings[fieldName])
.innerJoin(nestedTableName, `${tableName}.${fieldName}`, '=', `${nestedTableName}.id`);

query =
args.block !== undefined
? query.andWhereRaw(`${nestedTableName}.block_range @> int8(??)`, [args.block])
: query.andWhereRaw(`upper_inf(${nestedTableName}.block_range)`);
query = applyBlockFilter(query, nestedTableName, args.block);

handleWhere(query, nestedTableName, w[1]);
} else {
Expand Down Expand Up @@ -253,16 +247,10 @@ export const getNestedResolver =

let result: Record<string, any>[] = [];
if (!derivedFromDirective) {
let query = knex
.select('*')
.from(getTableName(columnName))
.whereIn('id', parent[info.fieldName]);
query =
block !== undefined
? query.andWhereRaw('block_range @> int8(??)', [block])
: query.andWhereRaw('upper_inf(block_range)');

result = await query;
const tableName = getTableName(columnName);
const query = knex.select('*').from(tableName).whereIn('id', parent[info.fieldName]);

result = await applyBlockFilter(query, tableName, block);
} else {
const fieldArgument = derivedFromDirective.arguments?.find(arg => arg.name.value === 'field');
if (!fieldArgument || fieldArgument.value.kind !== 'StringValue') {
Expand Down
2 changes: 2 additions & 0 deletions src/stores/checkpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export enum MetadataId {
SchemaVersion = 'schema_version'
}

export const INTERNAL_TABLES = Object.values(Table);

const CheckpointIdSize = 10;

/**
Expand Down
10 changes: 10 additions & 0 deletions src/utils/database.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import pluralize from 'pluralize';
import { Knex } from 'knex';
import { INTERNAL_TABLES } from '../stores/checkpoints';

export const getTableName = (name: string) => {
if (name === '_metadata') return '_metadatas';

return pluralize(name);
};

export function applyBlockFilter(query: Knex.QueryBuilder, tableName: string, block?: number) {
if (INTERNAL_TABLES.includes(tableName)) return query;

return block !== undefined
? query.andWhereRaw(`${tableName}.block_range @> int8(??)`, [block])
: query.andWhereRaw(`upper_inf(${tableName}.block_range)`);
}
57 changes: 57 additions & 0 deletions test/unit/utils/database.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import knex from 'knex';
import { getTableName, applyBlockFilter } from '../../../src/utils/database';

const mockKnex = knex({
client: 'sqlite3',
connection: {
filename: ':memory:'
},
useNullAsDefault: true
});

afterAll(async () => {
await mockKnex.destroy();
});

describe('getTableName', () => {
it.each([
['table', 'tables'],
['user', 'users'],
['post', 'posts'],
['space', 'spaces'],
['vote', 'votes'],
['comment', 'comments']
])('should return pluralized table name', (name, expected) => {
expect(getTableName(name)).toEqual(expected);
});

it('should return hardcoded table name for metadata', () => {
expect(getTableName('_metadata')).toEqual('_metadatas');
});
});

describe('applyBlockFilter', () => {
it('should not apply filter for internal tables', () => {
const query = mockKnex.select('*').from('_metadatas');

const result = applyBlockFilter(query, '_metadatas', 123);

expect(result.toString()).toBe('select * from `_metadatas`');
});

it('should apply capped block filter if block is provided', () => {
const query = mockKnex.select('*').from('posts');

const result = applyBlockFilter(query, 'posts', 123);

expect(result.toString()).toBe('select * from `posts` where posts.block_range @> int8(123)');
});

it('should apply upper_inf block filter if block is not provided', () => {
const query = mockKnex.select('*').from('posts');

const result = applyBlockFilter(query, 'posts');

expect(result.toString()).toBe('select * from `posts` where upper_inf(posts.block_range)');
});
});

0 comments on commit 333ca3f

Please sign in to comment.