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

[Drizzle] Add integration tests #1151

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

[Drizzle] Add integration tests #1151

wants to merge 5 commits into from

Conversation

SferaDev
Copy link
Member

@SferaDev SferaDev commented Sep 1, 2023

How to run them locally

  1. Create an .env file with XATA_API_KEY and XATA_WORKSPACE. If you want to connect somewhere different than production use XATA_API_PROVIDER and/or XATA_REGION.
  2. Run pnpm install
  3. In packages/plugin-client-drizzle run pnpm test
  4. Enable failing tests by removing .skip from test.skip(...)

Signed-off-by: Alexis Rico <sferadev@gmail.com>
@changeset-bot
Copy link

changeset-bot bot commented Sep 1, 2023

⚠️ No Changeset found

Latest commit: 5723a50

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

Your pull request has been published to npm.

You can install @xata.io/client by running:

npm install @xata.io/client@0.0.0-alpha.ve69ec6a

Other packages are published like this:

npm install @xata.io/mypackage@0.0.0-alpha.ve69ec6a

To test the CLI, run:

npx @xata.io/cli@0.0.0-alpha.ve69ec6a

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

size-limit report 📦

Path Size
packages/client/dist/index.mjs 15.81 KB (0%)
packages/client/dist/index.cjs 16.97 KB (0%)
packages/codegen/dist/index.mjs 1.89 MB (0%)
packages/codegen/dist/index.cjs 1.89 MB (0%)

Signed-off-by: Alexis Rico <sferadev@gmail.com>
Signed-off-by: Alexis Rico <sferadev@gmail.com>
Signed-off-by: Alexis Rico <sferadev@gmail.com>
name: text('name').notNull()
});

await db.execute(sql`drop table if exists ${usersDistinctTable}`);
Copy link
Contributor

@kvch kvch Oct 6, 2023

Choose a reason for hiding this comment

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

We do not support DDL statements like DROP TABLE as the error message suggests:

'invalid SQL: unsupported statement type, available: [SELECT, INSERT, UPDATE, DELETE]'

});

await db.execute(sql`drop table if exists ${usersDistinctTable}`);
await db.execute(sql`create table ${usersDistinctTable} (id integer, name text)`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not support DDL statements like CREATE TABLE as the error message suggests:

'invalid SQL: unsupported statement type, available: [SELECT, INSERT, UPDATE, DELETE]'

name: usersTable.name
});

t.deepEqual(users, [{ id: 1, name: 'Jane' }]);
Copy link
Contributor

@kvch kvch Oct 6, 2023

Choose a reason for hiding this comment

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

The id is not 1 but rather the Xata generated id that has a format rec_{randomstring}. Also, id is a read only field, we do not support users setting it.

await db.insert(usersTable).values({ name: 'John' });
const users = await db.update(usersTable).set({ name: 'Jane' }).where(eq(usersTable.name, 'John')).returning();

t.assert(users[0]!.createdAt instanceof Date);
Copy link
Contributor

@kvch kvch Oct 6, 2023

Choose a reason for hiding this comment

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

Xata contains the right timestamp. However, the timestamp format differs from the one we have for xata internal fields. For example, created_at is 2023-10-06T09:53:49.276Z but our internal xata.creadtedAt field contains 2023-10-06T09:53:49.276755Z (note the precision difference). Mayb that's why datetime parsing fails and we get invalid date errors?

t.deepEqual(result, [{ id: 1, name: 'Atlanta', state: 'GA' }]);
});

test.skip('char delete', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This passes for me.

await db.insert(usersTable).values({ name: 'John', verified: true });
const result = await db.select().from(usersTable);

t.deepEqual(result, [{ id: 1, name: 'John', verified: true, jsonb: null, createdAt: result[0]!.createdAt }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

The id is incorrect and createdAt cannot be parsed.

})
.from(usersTable);

t.deepEqual(result, [
Copy link
Contributor

Choose a reason for hiding this comment

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

ids are incorrect.

verified: usersTable.verified
});

t.deepEqual(result, [
Copy link
Contributor

Choose a reason for hiding this comment

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

ids are incorrect.

]);
});

test.skip('select with group by as field', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test passes for me locally. Also, I do not see anything problematic.

t.deepEqual(result, [{ name: 'Jane' }, { name: 'John' }]);
});

test.skip('select with group by as sql', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test passes for me locally. Also, I do not see anything problematic.

.from(usersTable)
.groupBy(sql`${usersTable.name}`, usersTable.id);

t.deepEqual(result, [{ name: 'Jane' }, { name: 'Jane' }, { name: 'John' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

As the id is not numeric, the ordering can be different. Locally I get John, Jane, Jane. We get the right elements, but in a different order.

.from(usersTable)
.groupBy(usersTable.id, sql`${usersTable.name}`);

t.deepEqual(result, [{ name: 'Jane' }, { name: 'Jane' }, { name: 'John' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ordering is different just like in the test above.

t.deepEqual(result, [{ name: 'Jane' }, { name: 'Jane' }, { name: 'John' }]);
});

test.skip('select with group by complex query', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It passes for me locally, and also it is supposed to work.


await db.insert(usersTable).values({ name: sql`${'John'}` });
const result = await db.select({ id: usersTable.id, name: usersTable.name }).from(usersTable);
t.deepEqual(result, [{ id: 1, name: 'John' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Id is incorrect.

.leftJoin(customerAlias, eq(customerAlias.id, 11))
.where(eq(usersTable.id, 10));

t.deepEqual(result, [
Copy link
Contributor

Choose a reason for hiding this comment

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

As the IDs are incorrect in the select, the result is empty.

name: text('name').notNull()
});

await db.execute(sql`drop table if exists ${users}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

DROP and CREATE table clauses are not allowed.

await db.insert(usersTable).values({ name: sql`'Jo h n'` });
const result = await db.select({ id: usersTable.id, name: usersTable.name }).from(usersTable);

t.deepEqual(result, [{ id: 1, name: 'Jo h n' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Id is incorrect.

.prepare('statement1');
const result = await statement.execute();

t.deepEqual(result, [{ id: 1, name: 'John' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Id is incorrect.

})
.from(usersTable);

t.deepEqual(result, [
Copy link
Contributor

Choose a reason for hiding this comment

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

Ids are incorrect.

})
.from(usersTable)
.where(eq(usersTable.id, placeholder('id')))
.prepare('stmt3');
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK we do not support named prepared statements.

});

// TODO change tests to new structure
test.skip('migrator', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

DDL statements are not supported.

test.skip('insert via db.execute + select via db.execute', async (t) => {
const { db } = t.context;

await db.execute(sql`insert into ${usersTable} (${name(usersTable.name.name)}) values (${'John'})`);
Copy link
Contributor

Choose a reason for hiding this comment

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

insert into users("name") values('John'); supposed to work. That is the statement that is executed, correct?

t.deepEqual(inserted.rows, [{ id: 1, name: 'John' }]);
});

test.skip('build query insert with onConflict do update', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be an issue with the query builder, not the SQL proxy.

});
});

test.skip('insert with onConflict do update', async (t) => {
Copy link
Contributor

@kvch kvch Oct 6, 2023

Choose a reason for hiding this comment

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

There is no conflict because id is not 1, so the record is not updated.

.from(usersTable)
.where(eq(usersTable.id, 1));

t.deepEqual(res, [{ id: 1, name: 'John' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason id is a string here. It's curious that we allow setting the id...

t.deepEqual(res, [{ id: 1, name: 'John' }]);
});

test.skip('left join (flat object fields)', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an issue in the SQL proxy. The query

select "users2"."id", "users2"."name", "cities"."id", "cities"."name" from "users2" left join "cities" on "users2"."city_id" = "cities"."id"

only returns

{
    "records":[
        {
            "id":"rec_ckfv2p5c8vb65ejtq8s0"
            "name":"Paris"
        },
        {
            "id":""
            "name":""
        }
    ]
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

]);
});

test.skip('left join (grouped fields)', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

.insert(courseCategoriesTable)
.values([{ name: 'Category 1' }, { name: 'Category 2' }, { name: 'Category 3' }, { name: 'Category 4' }]);

await db.insert(coursesTable).values([
Copy link
Contributor

Choose a reason for hiding this comment

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

We return a constraint violation because the category IDs are incorrect.

.select({
region: orders.region,
product: orders.product,
productUnits: sql<number>`sum(${orders.quantity})::int`,
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that aliasing is missing the query. We run sum("quantity")::int, sum("amount")::int, and productUnits and productSales is missing from the query.
So I guess it has to be adjusted in the adapter to make sure the sums are displayed with the correct name. Or we can alias the sum with the name in the query.

]);
});

test.skip('select from subquery sql', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is passing for me.


const res = await db.select({ count: sql`count(*)` }).from(usersTable);

t.deepEqual(res, [{ count: '2' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

We return an integer, not a string.

t.deepEqual(res, [{ count: '2' }]);
});

test.skip('select count w/ custom mapper', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test passes for me.

t.deepEqual(res, [{ count: 2 }]);
});

test.skip('network types', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test passes for me.


await db.insert(salEmp).values(values);

const res = await db.select().from(salEmp);
Copy link
Contributor

Choose a reason for hiding this comment

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

The backend returns the correct reponse:

{
  "records": [
    {
      "name": "John",
      "pay_by_quarter": [
        "10000",
        "10000",
        "10000",
        "10000"
      ],
      "schedule": [
        "meeting",
        "lunch",
        "training",
        "presentation"
      ]
    },
    {
      "name": "Carol",
      "pay_by_quarter": [
        "20000",
        "25000",
        "25000",
        "25000"
      ],
      "schedule": [
        "breakfast",
        "consulting",
        "meeting",
        "lunch"
      ]
    }
  ]
}

But I get the following JS error:

TypeError {
  message: 'Cannot read properties of undefined (reading \'map\')',
}

await db.insert(citiesTable).values([{ name: 'London' }, { name: 'Paris' }, { name: 'New York' }]);

await db.insert(users2Table).values([
{ name: 'John', cityId: 1 },
Copy link
Contributor

Choose a reason for hiding this comment

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

Id is incorrect so we return a constraint violation error.

cityId: integer('city_id').notNull()
}).existing();

await db.execute(sql`create view ${newYorkers1} as ${getViewConfig(newYorkers1).query}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not allow creating views.

cityId: integer('city_id').notNull()
}).existing();

await db.execute(sql`create materialized view ${newYorkers1} as ${getMaterializedViewConfig(newYorkers1).query}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not allow creating materialized views.


Expect<Equal<{ userId: number; name: string; userCity: string; cityId: number; cityName: string }[], typeof result>>;

t.deepEqual(result, [{ userId: 1, name: 'John', userCity: 'New York', cityId: 1, cityName: 'Paris' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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


Expect<Equal<{ userId: number; name: string; userCity: string; cityId: number; cityName: string }[], typeof result>>;

t.deepEqual(result, [{ userId: 1, name: 'John', userCity: 'New York', cityId: 1, cityName: 'Paris' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Both incorrect ID and the known issue we have: https://github.com/xataio/xata/issues/2572

name: text('name').notNull()
});

await db.execute(sql`drop table if exists ${users}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

No DROP TABLE support in Xata.

await db.execute(sql`drop table if exists ${users}`);

await db.execute(
sql`create table myprefix_test_prefixed_table_with_unique_name (id integer not null primary key, name text not null)`
Copy link
Contributor

Choose a reason for hiding this comment

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

No CREATE TABLE support in Xata.

.default(sql`now()`)
});

await db.execute(sql`drop table if exists ${exercises}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

No DROP TABLE

createdAt: timestamp('created_at').notNull()
});

await db.execute(sql`drop table if exists ${metricEntry}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

No DROP TABLE and CREATE TABLE support in Xata.


await db.execute(
sql`
create table users_test_with_and_without_timezone (
Copy link
Contributor

Choose a reason for hiding this comment

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

No CREATE TABLE.

.returning()
.then((rows) => rows[0]!);

await db.transaction(async (tx) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not support transactions.

await db.execute(sql`drop table ${products}`);
});

test.skip('transaction rollback', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not support transaction rollback.

Copy link
Contributor

@kvch kvch left a comment

Choose a reason for hiding this comment

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

I went through all of the tests in the pull request. There is one issue that has to be fixed in the proxy: https://github.com/xataio/xata/issues/2572

Other reasons of test failures:

  • Timestamp format differs from what we support, so the test fails to parse the timestamp from PostgreSQL
  • When running functions PostgreSQL uses the function name as the column name, but the tests expect it to be the column name
  • DDL statements and transactions are not supported
  • The tests all try to set the ID for the records, but we return our own IDs. When the tests tries to use the bad ID, we return nothing, as the ID is unknown to us.
  • We use our IDs in linked tables, so the tests' IDs do not work

So on the SQL proxy side we have to fix the known issue, and maybe rethink how we handle the IDs. Should we get rid of our ID format?
Also, we will support more DDL commands in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants