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

fix(questdb-driver): schema query error due to system tables #4762

Merged
merged 2 commits into from Jun 22, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Expand Up @@ -86,7 +86,7 @@ https://github.com/cube-js/cube.js/blob/master/packages/cubejs-jdbc-driver/READM

#### Development

Cube.js is written in a mixture of plain JavaScript and Typescript. TypeScript is preferred for new code.
Cube.js is written in a mixture of plain JavaScript and TypeScript. TypeScript is preferred for new code.

> Attention: Cube.js uses TypeScript configured in incremental mode, which uses cache to speed up compilation,
> but in some cases, you can run into a problem with a not recompiled file. To fix it, we recommend running `$ yarn clean` and `$ yarn tsc`.
Expand Down
8 changes: 6 additions & 2 deletions packages/cubejs-questdb-driver/src/QuestDriver.ts
Expand Up @@ -145,6 +145,10 @@ export class QuestDriver<Config extends QuestDriverConfiguration = QuestDriverCo
if (tableName === undefined) {
return;
}
// Skip system tables.
if (tableName.startsWith('sys.')) {
return;
}
const columns = await this.tableColumnTypes(tableName);
metadata[''][tableName] = columns;
}));
Expand All @@ -162,7 +166,7 @@ export class QuestDriver<Config extends QuestDriverConfiguration = QuestDriverCo
}

public async tableColumnTypes(table: string) {
const response: any[] = await this.query(`SHOW COLUMNS FROM ${table}`, []);
const response: any[] = await this.query(`SHOW COLUMNS FROM '${table}'`, []);

return response.map((row) => ({ name: row.column, type: this.toGenericType(row.type) }));
}
Expand All @@ -182,7 +186,7 @@ export class QuestDriver<Config extends QuestDriverConfiguration = QuestDriverCo
try {
for (let i = 0; i < tableData.rows.length; i++) {
await this.query(
`INSERT INTO ${table}
`INSERT INTO '${table}'
(${columns.map(c => this.quoteIdentifier(c.name)).join(', ')})
VALUES (${columns.map((c, paramIndex) => this.param(paramIndex)).join(', ')})`,
columns.map(c => this.toColumnValue(tableData.rows[i][c.name], c.type))
Expand Down
63 changes: 60 additions & 3 deletions packages/cubejs-questdb-driver/test/QuestDriver.test.ts
Expand Up @@ -49,9 +49,9 @@ describe('QuestDriver', () => {
const tableData = await driver.query('select * from query_test', []);

expect(tableData).toEqual([
{ id: "1", created: '2020-01-01T00:00:00.000', price: 100.5 },
{ id: "2", created: '2020-01-02T00:00:00.000', price: 200.5 },
{ id: "3", created: '2020-01-03T00:00:00.000', price: 300.5 }
{ id: '1', created: '2020-01-01T00:00:00.000', price: 100.5 },
{ id: '2', created: '2020-01-02T00:00:00.000', price: 200.5 },
{ id: '3', created: '2020-01-03T00:00:00.000', price: 300.5 }
]);
});

Expand All @@ -66,4 +66,61 @@ describe('QuestDriver', () => {
);
}
});

test('schema', async () => {
const schema = await driver.tablesSchema();

expect(schema['']).toEqual({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are the tables query_test, telemetry and telemetry_config created?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query_test comes from the test('query', ...) test. telemetry and telemetry_config are built-in tables created for the purposes of telemetry events: https://questdb.io/docs/reference/configuration/#telemetry

'query_test': [
{
name: 'id',
type: 'LONG',
},
{
name: 'created',
type: 'date',
},
{
name: 'price',
type: 'DOUBLE',
},
],
'telemetry': [
{
name: 'created',
type: 'TIMESTAMP',
},
{
name: 'event',
type: 'SHORT',
},
{
name: 'origin',
type: 'SHORT',
},
],
'telemetry_config': [
{
name: 'id',
type: 'LONG256',
},
{
name: 'enabled',
type: 'boolean',
},
{
name: 'version',
type: 'SYMBOL',
},
{
name: 'os',
type: 'SYMBOL',
},
{
name: 'package',
type: 'SYMBOL',
},
],
});
});
});
2 changes: 1 addition & 1 deletion packages/cubejs-testing-shared/src/db/questdb.ts
Expand Up @@ -8,7 +8,7 @@ type QuestStartOptions = DBRunnerContainerOptions & {

export class QuestDBRunner extends DbRunnerAbstract {
public static startContainer(options: QuestStartOptions) {
const version = process.env.TEST_QUEST_DB_VERSION || options.version || '6.2.1';
const version = process.env.TEST_QUEST_DB_VERSION || options.version || '6.4.1';

const container = new GenericContainer(`questdb/questdb:${version}`)
.withExposedPorts(8812)
Expand Down
2 changes: 1 addition & 1 deletion packages/cubejs-testing/DEVELOPMENT.md
Expand Up @@ -53,7 +53,7 @@ $ export BIRDBOX_CUBEJS_REGISTRY_PATH=localhost:5000/
$ export BIRDBOX_CYPRESS_BROWSER=chrome
$ export BIRDBOX_CYPRESS_TARGET=postgresql
$ export DEBUG=testcontainers
$ yarn database:minimal
$ yarn dataset:minimal
$ yarn cypress:install
$ yarn cypress:birdbox
```
Expand Down
Expand Up @@ -3,7 +3,7 @@
exports[`questdb query measure: query 1`] = `
Array [
Object {
"Orders.totalAmount": "114",
"Orders.totalAmount": "1700",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat nervous, this test snapshot changed without an obvious corresponding change in the test spec. Is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has to do with QuestDB version upgrade in integration tests. There was a bug in the query engine around aggregated over UNION ALL which was fixed recently. If you check packages/cubejs-testing/birdbox-fixtures/materialize/schema/Orders.js, you'll notice that 1700 is the correct value.

},
]
`;
5 changes: 5 additions & 0 deletions yarn.lock
Expand Up @@ -7083,6 +7083,11 @@
resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-8.3.3.tgz#c6a60686d953dbd1b1d45e66f4ecdbd5d471b4d0"
integrity sha512-0LbEEx1zxrYB3pgpd1M5lEhLcXjKJnYghvhTRgaBeUivLHMDM1TzF3IJ6hXU2+8uA4Xz+5BA63mtZo5DjVT8iA==

"@types/uuid@^8.3.4":
version "8.3.4"
resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-8.3.4.tgz#bd86a43617df0594787d38b735f55c805becf1bc"
integrity sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==

"@types/webpack-dev-server@^3.11.0":
version "3.11.6"
resolved "https://registry.yarnpkg.com/@types/webpack-dev-server/-/webpack-dev-server-3.11.6.tgz#d8888cfd2f0630203e13d3ed7833a4d11b8a34dc"
Expand Down