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

Materialized View does not enforce columns names - 1.13.0 #1545

Closed
danthegoodman1 opened this issue Apr 11, 2022 · 4 comments
Closed

Materialized View does not enforce columns names - 1.13.0 #1545

danthegoodman1 opened this issue Apr 11, 2022 · 4 comments

Comments

@danthegoodman1
Copy link

Version

Other

What happened?

When compiling, sqlc does not verify that the columns exist in the mat view.

See the schema below and the query, the query compiles fine, but running that on the DB throws column "earned" does not exist

Relevant log output

No response

Database schema

CREATE TABLE kpis (
  ts TIMESTAMPTZ,
  event_id TEXT NOT NULL,
  svc TEXT NOT NULL,
  stage TEXT,
  region TEXT,
  instance TEXT,
  player_id TEXT,
  slip_id TEXT,
  game TEXT,
  data JSONB NOT NULL
);

CREATE MATERIALIZED VIEW IF NOT EXISTS ten_day_kills
WITH (timescaledb.continuous) AS
SELECT time_bucket(INTERVAL '10 day', ts) AS bucket,
  COUNT(*) AS kill_count,
  (data->>'winning_player_id')::TEXT pid
FROM kpis
WHERE event_id = 'player.defeated'
GROUP BY pid, bucket
;

SQL queries

SELECT COALESCE(SUM(earned), 0) as total_earned
FROM ten_day_kills
WHERE pid = @player_id
;

Configuration

version: 1
packages:
  - path: "tsdb"
    name: "tsdb"
    engine: "postgresql"
    schema: "schema.sql"
    queries: "./queries/"
    sql_package: "pgx/v4"

Playground URL

https://play.sqlc.dev/p/5fa2f34d645dba3e876ff1d17c42a8927cfcb1b3885755abba3604831d8e1010

What operating system are you using?

macOS

What database engines are you using?

PostgreSQL

What type of code are you generating?

Go

@danthegoodman1 danthegoodman1 added bug Something isn't working triage New issues that hasn't been reviewed labels Apr 11, 2022
@danthegoodman1
Copy link
Author

danthegoodman1 commented Apr 11, 2022

The following also compiles, even though total_ear is not defined

SELECT SUM(earned) as total_earned,
pid
FROM ten_day_earnings
GROUP BY pid
ORDER BY total_ear DESC
LIMIT 10;

@danthegoodman1
Copy link
Author

It also seems like it doesn't enforce not null checks on aliased columns. The first MV below has pid as a required query param, but the second has it as a sql.NullString

CREATE MATERIALIZED VIEW IF NOT EXISTS ten_day_earnings
WITH (timescaledb.continuous) AS
SELECT time_bucket(INTERVAL '10 day', ts) AS bucket,
  (data->>'winning_player_id')::TEXT  AS pid,
  SUM((data->>'tokens_gained')::INT8) AS earned
FROM kpis
WHERE event_id = 'player.defeated'
AND (data::JSON->>'player_id') = player_id
GROUP BY pid, bucket
;

CREATE MATERIALIZED VIEW IF NOT EXISTS ten_day_losings
WITH (timescaledb.continuous) AS
SELECT time_bucket(INTERVAL '10 day', ts) AS bucket,
  player_id AS pid,
  SUM((data->>'lobby_price')::INT8) AS lost
FROM kpis
WHERE event_id = 'player.defeated'
AND (data::JSON->>'player_id') = player_id
GROUP BY pid, bucket
;

@danthegoodman1
Copy link
Author

@kyleconroy just wanted to bump this as running into this is pretty painful, since I otherwise assume sqlc guarantees correctness :)

@kyleconroy kyleconroy added 📚 postgresql 💻 darwin 🔧 golang and removed triage New issues that hasn't been reviewed labels Jun 5, 2022
kyleconroy added a commit that referenced this issue Oct 17, 2023
kyleconroy added a commit that referenced this issue Oct 18, 2023
* fix(compiler): Pull in array information from analyzer
Fixes #1532
* test(analyzer): Add testcase for #1574
* test: Added test for #1634
* test: Add test case for #1646
* test: Add test for #1714
* Fixes #1912
* test: Add case for #1916
* test: Add two test cases
#1917
#1545
* test: Add case for #1979
* test: Add case for #1990
@kyleconroy
Copy link
Collaborator

This is fixed in v1.23.0 by enabling the database-backed query analyzer. We added a test case for this issue so it won’t break in the future.

You can play around with the working example on the playground

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

2 participants