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

Server/CockroachDB: Verify Metadata API for CockroachDB #8799

Closed
Tracked by #8788
sassela opened this issue Aug 16, 2022 · 7 comments
Closed
Tracked by #8788

Server/CockroachDB: Verify Metadata API for CockroachDB #8799

sassela opened this issue Aug 16, 2022 · 7 comments

Comments

@sassela
Copy link
Contributor

sassela commented Aug 16, 2022

This issue involves:

  • at a minimum verifying manually that adding a cockroachDB via the cockroach_add_source operation works (with the same inputs as the pg_add_source operation)
  • verify that cockroach_track_tables works
  • optionally, adding a hspec test for replace_metadata with a cockroachDB
  • optionally, adding a hspec test for cockroach_add_source
@danieljharvey
Copy link
Contributor

This may work already due to the boilerplate changes, needs testing though.

@sassela sassela changed the title server/cockroachdb: support Metadata API server/cockroachdb: verify Metadata API for cockroachDB Aug 23, 2022
@plcplc
Copy link
Contributor

plcplc commented Aug 23, 2022

To relate this issue with our initial scope as defined in #8788, we could test:

  • adding/dropping a source
  • un-/tracking a table
  • un-/tracking a relationship
  • setting permissions on a table.

... as these are the commands we're going to rely on for that scope.

For some technical context, the implementation of the metadata API commands is backend-generic and simply a matter of enabling/disabling, c.f.:

https://github.com/hasura/graphql-engine-mono/blob/5d45c8ac85aa3721697f32cd7910a7e648d130f6/server/src-lib/Hasura/Backends/Postgres/Instances/API.hs

As such it might not be of much interest to test these thoroughly for each backend.
On the one hand, any later test of actual features is going to subsume this. But on the other hand, a specific metadata api test could be a useful debugging clue when feature tests fail and the root cause happens to be a fault in the metadata api implementation.

@soupi
Copy link
Contributor

soupi commented Aug 23, 2022

Gave a quick try running cockroach_add_source.

  1. I'm using docker compose up to start a cockroach database (from this branch)

  2. I set export COCKROACH_DB="postgresql://[email protected]:65008/defaultdb?sslmode=disable"

  3. I create a file /tmp/addsource.json with the following content:

    /tmp/addsource.json
    {
      "type": "cockroach_add_source",
      "args": {
        "name": "cockroach",
        "configuration": {
          "connection_info": {
            "database_url": {
               "from_env": "COCKROACH_DB"
             },
            "pool_settings": {
              "max_connections": 50,
              "idle_timeout": 180,
              "retries": 1,
              "pool_timeout": 360,
              "connection_lifetime": 600
            },
            "use_prepared_statements": true,
            "isolation_level": "read-committed"
          }
        }
      }
    }
  4. I started graphql-engine and ran the command: cat /tmp/addsource.json | curl -d @- localhost:8181/v1/metadata

I'm currently getting the error:

error
{
  "detail": {
    "http_info": {
      "content_encoding": null,
      "http_version": "HTTP/1.1",
      "ip": "127.0.0.1",
      "method": "POST",
      "status": 400,
      "url": "/v1/metadata"
    },
    "operation": {
      "error": {
        "code": "invalid-configuration",
        "error": "Inconsistent object: database query error",
        "internal": [
          {
            "definition": "cockroach",
            "message": {
              "arguments": [
                "(Oid 114,Just (\"[]\",Binary))"
              ],
              "error": {
                "description": null,
                "exec_status": "FatalError",
                "hint": null,
                "message": "received too many type hints: 1 vs 0 placeholders in query",
                "status_code": "08P01"
              },
              "prepared": true,
              "statement": "WITH\n  \"tabletable\" as ( SELECT \"table\".oid,\n           \"table\".relkind,\n           \"table\".relname AS \"table_name\",\n           \"schema\".nspname AS \"table_schema\"\n      FROM pg_catalog.pg_class \"table\"\n      JOIN pg_catalog.pg_namespace \"schema\"\n          ON schema.oid = \"table\".relnamespace\n      WHERE \"table\".relkind IN ('r', 't', 'v', 'm', 'f', 'p')\n        -- and tables not from any system schemas\n        AND \"schema\".nspname NOT LIKE 'pg_%'\n        AND \"schema\".nspname NOT IN ('information_schema', 'hdb_catalog', 'hdb_lib', '_timescaledb_internal', 'crdb_internal')\n  )\nSELECT\n  \"table\".table_schema,\n  \"table\".table_name,\n\n  -- This field corresponds to the `DBTableMetadata` Haskell type\n  jsonb_build_object(\n    'oid', \"table\".oid :: integer,\n    'columns', coalesce(columns.info, '[]'),\n    'primary_key', primary_key.info,\n    -- Note: unique_constraints does NOT include primary key constraints!\n    'unique_constraints', coalesce(unique_constraints.info, '[]'),\n    'foreign_keys', coalesce(foreign_key_constraints.info, '[]'),\n    -- Note: for views and materialized views, we are asking Postgres if it is mutable or not\n    --       and for any other case, we are assuming it to be mutable.\n    'view_info', CASE WHEN \"table\".relkind IN ('v', 'm') THEN jsonb_build_object(\n      'is_updatable', ((pg_catalog.pg_relation_is_updatable(\"table\".oid, true) & 4) = 4),\n      'is_insertable', ((pg_catalog.pg_relation_is_updatable(\"table\".oid, true) & 8) = 8),\n      'is_deletable', ((pg_catalog.pg_relation_is_updatable(\"table\".oid, true) & 16) = 16)\n    ) END,\n    'description', description.description,\n    'extra_table_metadata', '[]'::json\n  )::json AS info\nFROM \"tabletable\" \"table\"\n\n-- tracked tables\n-- $1 parameter provides JSON array of tracked tables\n-- FROM\n--   ( SELECT \"tracked\".\"name\" AS \"table_name\",\n--            \"tracked\".\"schema\" AS \"table_schema\"\n--       FROM jsonb_to_recordset($1::jsonb) AS \"tracked\"(\"schema\" text, \"name\" text)\n--   ) \"tracked_table\"\n\n-- table & schema\n-- LEFT JOIN\n--  ( SELECT \"table\".oid,\n--           \"table\".relkind,\n--           \"table\".relname AS \"table_name\",\n--           \"schema\".nspname AS \"table_schema\"\n--      FROM pg_catalog.pg_class \"table\"\n--      JOIN pg_catalog.pg_namespace \"schema\"\n--          ON schema.oid = \"table\".relnamespace\n--  ) \"table\"\n  --ON  \"table\".\"table_name\" = \"tracked_table\".\"table_name\"\n  --AND \"table\".\"table_schema\" = \"tracked_table\".\"table_schema\"\n\n-- description\nLEFT JOIN pg_catalog.pg_description description\n  ON  description.classoid = 'pg_catalog.pg_class'::regclass\n  AND description.objoid = \"table\".oid\n  AND description.objsubid = 0\n\n-- columns\nLEFT JOIN LATERAL\n  ( SELECT jsonb_agg(jsonb_build_object(\n      'name', \"column\".attname,\n      'position', \"column\".attnum,\n      'type', json_build_object('name', coalesce(base_type.typname, \"type\".typname), 'type', \"type\".typtype),\n      'is_nullable', NOT \"column\".attnotnull,\n      'description', pg_catalog.col_description(\"table\".oid, \"column\".attnum),\n      'mutability', jsonb_build_object(\n        'is_insertable', NOT (identitypolyfill.attidentity = 'a' OR generatedpolyfill.attgenerated = 's'),\n        'is_updatable', NOT (identitypolyfill.attidentity = 'a' OR generatedpolyfill.attgenerated = 's'))\n    )) AS info\n    FROM pg_catalog.pg_attribute \"column\"\n\n    -- The columns 'pg_attribute.attidentity' and 'pg_attribute.attgenerated' are\n    -- not available in older versions of Postgres, because those versions do not\n    -- implement the concepts the catalog columns represent.\n    -- To support older versions we apply an aliasing hack that ensures\n    -- _something_ called e.g. attidentity is in scope.\n    -- Originally sourced from: https://stackoverflow.com/questions/18951071/postgres-return-a-default-value-when-a-column-doesnt-exist.\n    INNER JOIN\n    (\n      SELECT attrelid, attnum, attname, CASE WHEN attidentity_exists\n                                        THEN attidentity::text\n                                        ELSE ''::text\n                                        END as attidentity\n      FROM pg_catalog.pg_attribute\n      CROSS JOIN (SELECT current_setting('server_version_num')::int >= 100000)\n            AS attidentity(attidentity_exists)\n    ) AS identitypolyfill\n      ON  identitypolyfill.attrelid = \"column\".attrelid\n      AND identitypolyfill.attnum = \"column\".attnum\n      AND identitypolyfill.attname = \"column\".attname\n\n    INNER JOIN\n    (\n      SELECT attrelid, attnum, attname, CASE WHEN attgenerated_exists\n                                                 THEN attgenerated::text\n                                                 ELSE ''::text\n                                                 END as attgenerated\n      FROM pg_catalog.pg_attribute\n      CROSS JOIN (SELECT current_setting('server_version_num')::int >= 120000)\n            AS attgenerated(attgenerated_exists)\n    ) AS generatedpolyfill\n      ON  generatedpolyfill.attrelid = \"column\".attrelid\n      AND generatedpolyfill.attnum = \"column\".attnum\n      AND generatedpolyfill.attname = \"column\".attname\n\n    LEFT JOIN pg_catalog.pg_type \"type\"\n      ON \"type\".oid = \"column\".atttypid\n    LEFT JOIN pg_catalog.pg_type base_type\n      ON \"type\".typtype = 'd' AND base_type.oid = \"type\".typbasetype\n    WHERE \"column\".attrelid = \"table\".oid\n      -- columns where attnum <= 0 are special, system-defined columns\n      AND \"column\".attnum > 0\n      -- dropped columns still exist in the system catalog as \"zombie\" columns, so ignore those\n      AND NOT \"column\".attisdropped\n  ) columns ON true\n\n-- primary key\nLEFT JOIN LATERAL\n  ( SELECT jsonb_build_object(\n      'constraint', jsonb_build_object(\n        'name', class.relname,\n        'oid', class.oid :: integer),\n      'columns', coalesce(columns.info, '[]')\n    ) AS info\n    FROM pg_catalog.pg_index idx\n    JOIN pg_catalog.pg_class class\n      ON class.oid = idx.indexrelid\n    LEFT JOIN LATERAL\n      ( SELECT jsonb_agg(\"column\".attname) AS info\n        FROM pg_catalog.pg_attribute \"column\"\n        WHERE \"column\".attrelid = \"table\".oid\n          AND \"column\".attnum = ANY (idx.indkey)\n      ) AS columns ON true\n    WHERE idx.indrelid = \"table\".oid\n      AND idx.indisprimary\n  ) primary_key ON true\n\n-- unique constraints\nLEFT JOIN LATERAL\n  ( SELECT jsonb_agg(\n      jsonb_build_object(\n        'constraint', jsonb_build_object(\n          'name', class.relname,\n          'oid', class.oid :: integer\n          ),\n        'columns', columns.info\n        )\n      ) AS info\n    FROM pg_catalog.pg_index idx\n    JOIN pg_catalog.pg_class class\n      ON class.oid = idx.indexrelid\n    LEFT JOIN LATERAL\n      ( SELECT jsonb_agg(\"column\".attname) AS info\n        FROM pg_catalog.pg_attribute \"column\"\n        WHERE \"column\".attrelid = \"table\".oid\n          AND \"column\".attnum = ANY (idx.indkey)\n      ) AS columns ON true\n    WHERE idx.indrelid = \"table\".oid\n      AND idx.indisunique\n      AND NOT idx.indisprimary\n  ) unique_constraints ON true\n\n-- foreign keys\nLEFT JOIN\n  ( SELECT jsonb_agg(jsonb_build_object(\n      'constraint', jsonb_build_object(\n        'name', foreign_key.constraint_name,\n        'oid', foreign_key.constraint_oid :: integer\n      ),\n      'columns', foreign_key.columns,\n      'foreign_table', jsonb_build_object(\n        'schema', foreign_key.ref_table_table_schema,\n        'name', foreign_key.ref_table\n      ),\n      'foreign_columns', foreign_key.ref_columns\n    )) AS info, -- This field corresponds to the `PGForeignKeyMetadata` Haskell type\n    foreign_key.table_schema,\n    foreign_key.table_name\n    FROM (SELECT\n             q.table_schema :: text,\n             q.table_name :: text,\n             q.constraint_name :: text,\n             min(q.constraint_oid) :: integer as constraint_oid,\n             min(q.ref_table_table_schema) :: text as ref_table_table_schema,\n             min(q.ref_table) :: text as ref_table,\n             json_agg(ac.attname) as columns,\n             json_agg(afc.attname) as ref_columns\n         FROM\n             (SELECT\n                 ctn.nspname AS table_schema,\n                 ct.relname AS table_name,\n                 r.conrelid AS table_id,\n                 r.conname as constraint_name,\n                 r.oid as constraint_oid,\n                 cftn.nspname AS ref_table_table_schema,\n                 cft.relname as ref_table,\n                 r.confrelid as ref_table_id,\n                 UNNEST (r.conkey) AS column_id,\n                 UNNEST (r.confkey) AS ref_column_id\n              FROM\n                  pg_catalog.pg_constraint r\n                  JOIN pg_catalog.pg_class ct\n                    ON r.conrelid = ct.oid\n                  JOIN pg_catalog.pg_namespace ctn\n                    ON ct.relnamespace = ctn.oid\n                  JOIN pg_catalog.pg_class cft\n                    ON r.confrelid = cft.oid\n                  JOIN pg_catalog.pg_namespace cftn\n                    ON cft.relnamespace = cftn.oid\n              WHERE\n                  r.contype = 'f'\n             ) q\n             JOIN pg_catalog.pg_attribute ac\n               ON q.column_id = ac.attnum\n                  AND q.table_id = ac.attrelid\n             JOIN pg_catalog.pg_attribute afc\n               ON q.ref_column_id = afc.attnum\n                  AND q.ref_table_id = afc.attrelid\n         GROUP BY q.table_schema, q.table_name, q.constraint_name\n    ) foreign_key\n    GROUP BY foreign_key.table_schema, foreign_key.table_name\n  ) foreign_key_constraints\n    ON \"table\".table_name = foreign_key_constraints.table_name\n       AND \"table\".table_schema = foreign_key_constraints.table_schema;\n\n-- -- all these identify table-like things\n-- WHERE \"table\".relkind IN ('r', 't', 'v', 'm', 'f', 'p')\n--   -- and tables not from any system schemas\n--   AND \"table\".table_schema NOT LIKE 'pg_%'\n--   AND \"table\".table_schema NOT IN ('information_schema', 'hdb_catalog', 'hdb_lib', '_timescaledb_internal', 'crdb_internal');\n\n"
            },
            "name": "source cockroach",
            "reason": "Inconsistent object: database query error",
            "type": "source"
          }
        ],
        "path": "$.args"
      },
      "request_id": "2a3a428a-186e-48a1-84ea-1df197710805",
      "request_mode": "error",
      "response_size": 10833,
      "user_vars": {
        "x-hasura-role": "admin"
      }
    },
    "request_id": "2a3a428a-186e-48a1-84ea-1df197710805"
  },

@plcplc
Copy link
Contributor

plcplc commented Aug 23, 2022

I'm currently getting the error:

Either fetchTableMetadata needs to be made pgKind-aware such that it won't pass the list of known tables as a parameter as the current cockroach_table_metadata.sql doesn't use them, or cockroach_table_metadata.sql needs to be amended to use them - preferably by re-introducing the optimization disabled by a lack of jsonb_to_recordset function. Alternatively we can just use it in a dead WITH-bound definition or something.

@sassela
Copy link
Contributor Author

sassela commented Aug 24, 2022

@plcplc
Copy link
Contributor

plcplc commented Aug 24, 2022

This is also (likely) blocked by #8840

@robertjdominguez robertjdominguez changed the title server/cockroachdb: verify Metadata API for cockroachDB Server/CockroachDB: Verify Metadata API for CockroachDB Aug 25, 2022
@danieljharvey
Copy link
Contributor

Have tested cockroach_add_source ,replace_metadata, cockroach_track_tables and cockroach_untrack_tables, all are working.

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

No branches or pull requests

6 participants