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

Postico crashes with CockroachDB 2.1 rc-1 #600

Closed
awoods187 opened this Issue Oct 25, 2018 · 11 comments

Comments

Projects
None yet
8 participants
@awoods187

awoods187 commented Oct 25, 2018

cockroachdb/cockroach#31834 (comment)

CockroachDB used to work with Postico.

We suspect that Postico now attempts to take advantage of Cockroach unique show columns instead of using Postgres standard. Postico should use the Postgres standard approach rather than CockroachDB specific implementation in this case.

@jordanlewis

This comment has been minimized.

jordanlewis commented Oct 25, 2018

A little more detail:

CockroachDB changed the schema of its SHOW COLUMNS output recently. Those commands should be viewed as "porcelain", not "plumbing", a la Git commands - their schemas might change from release to release.

Postico should stick with the ordinary Postgres way of determining things - use pg_catalog.

@jakob

This comment has been minimized.

Owner

jakob commented Oct 28, 2018

Thanks for letting us know!

Unfortunately it's not as easy as disabling the special handling of CockroachDB, since Cockroach doesn't seem to support the queries we use to query pg_catalog.

Here's one of the queries that trips up CockroachDB if I remove the special handling:

SELECT
    pg_class.oid,
    pg_class.relname,
    indisunique,
    indisprimary,
    indisexclusion,
    indkey,
    pg_get_indexdef(indexrelid, 0, TRUE) AS definition,
    ARRAY (
        SELECT
            pg_get_indexdef(indexrelid, attnum, TRUE)
        FROM
            pg_attribute
        WHERE
            attrelid = indexrelid
        ORDER BY
            attnum) AS expressions,
    obj_description(pg_class.oid, 'pg_class') AS COMMENT,
    indoption,
    ARRAY (
        SELECT
            pg_collation.collname
        FROM
            unnest(indcollation) AS t (colid)
            LEFT JOIN pg_collation ON pg_collation.oid = colid) AS collations,
    ARRAY (
        SELECT
            pg_opclass.opcname
        FROM
            generate_series(0, indnatts - 1) AS t (i)
            LEFT JOIN pg_opclass ON pg_opclass.oid = indclass [ i ]) AS opclasses,
    pg_get_expr(indpred, indrelid, TRUE),
    amname
FROM
    pg_index
    LEFT JOIN pg_class ON pg_class.oid = indexrelid
    LEFT JOIN pg_am ON pg_class.relam = pg_am.oid
WHERE
    indrelid = '"public"."test"'::regclass
ORDER BY
    pg_class.oid;

We probably need to find a different way to write that query.

@RaduBerinde

This comment has been minimized.

RaduBerinde commented Oct 29, 2018

The error for the query above (I replaced indexrelid, indnatts, indcolation with some bogus values): virtual schema table not implemented: pg_catalog.pg_opclass

@bobvawter

This comment has been minimized.

bobvawter commented Oct 29, 2018

Tracking issue for pg_opclass: cockroachdb/cockroach#27763

@jakob

This comment has been minimized.

Owner

jakob commented Oct 29, 2018

I've now changed Postico to query pg_catalog instead of SHOW COLUMNS for CockroachDB, and I've made it a bit more resilient to schema changes (Postico really shouldn't crash for unexpected server inputs).

I've run our test suite against CockroachDB 2.0.0 and 2.1.0-rc.1. Everything looks good now.

(I see that the '\' LIKE '%\\%' is finally fixed in CockroachDB 2.1 👍)

Please try this build: postico-3700.zip

@vilterp

This comment has been minimized.

vilterp commented Oct 29, 2018

Works for me 👍 Thanks!

@aaronkramer

This comment has been minimized.

aaronkramer commented Oct 31, 2018

Confirmed - this build works for me also

@RaduBerinde

This comment has been minimized.

RaduBerinde commented Oct 31, 2018

(I see that the '\' LIKE '%\\%' is finally fixed in CockroachDB 2.1)

@jakob do you have more info on this? I can't think of any fix for LIKE we made in 2.1 (CC @knz)

@knz

This comment has been minimized.

knz commented Oct 31, 2018

@jakob

This comment has been minimized.

Owner

jakob commented Nov 5, 2018

@RaduBerinde
Here's a test query:

SELECT '\' LIKE '%\\', '\' LIKE '\\%', '\' LIKE '%\\%';

CRDB 2.0 returns: TRUE, TRUE, FALSE

CRDB 2.1 returns: TRUE, TRUE, TRUE

A similar query is executed as part of the Postico Table Filter test suite, so those tests always failed on CockroachDB. I'm happy that we have one less failing test case now :)

@jakob

This comment has been minimized.

Owner

jakob commented Nov 7, 2018

Postico 1.4.3 is now released, and our regression tests run successfully against CockroachDB 2.0 and 2.1.

@jakob jakob closed this Nov 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment