Skip to content

fix: correct ShopifyQL schema discovery and timestamp handling#153

Open
naveen-meltano wants to merge 2 commits into
masterfrom
fix/shopifyql-schema-and-timestamp-bugs
Open

fix: correct ShopifyQL schema discovery and timestamp handling#153
naveen-meltano wants to merge 2 commits into
masterfrom
fix/shopifyql-schema-and-timestamp-bugs

Conversation

@naveen-meltano

Copy link
Copy Markdown
Contributor

Summary

The sales_over_time stream (and any other config-driven ShopifyQLStream) was silently broken end-to-end. Found and fixed while debugging a real pipeline run — extraction/discovery reported success with zero errors while the destination table stayed empty.

  • Missing GraphQL data envelope unwrap: schema and parse_response both read data.get("shopifyqlQuery") instead of unwrapping the standard GraphQL data envelope ({"data": {"shopifyqlQuery": {...}}}). Every response silently resolved to {}, no error raised. This was a regression from an earlier PR review round that incorrectly claimed this endpoint has no envelope.
  • primary_keys/replication_key set before super().__init__(): these are properties backed by self._primary_keys/self._replication_key on the singer_sdk base Stream class, whose __init__ unconditionally resets both to their defaults. Since the subclass set them before calling super().__init__(), the configured values were silently wiped every time.
  • Replication key typed as plain StringType: the discovered schema types every column as StringType (values are cast downstream in dbt), but the replication key column itself needs a real timestamp-typed schema (format: date-time) for singer_sdk's bookmark logic — otherwise it raises InvalidReplicationKeyException / ValueError: not of timestamp type.
  • Date-only replication key values: ShopifyQL returns date-only strings (e.g. "2026-07-03") for TIMESERIES columns. Once typed as a timestamp, this fails downstream TIMESTAMP-typed destination columns (e.g. BigQuery rejects date-only strings for TIMESTAMP load). Normalized in post_process by appending T00:00:00Z when the value is a bare date.

These four bugs compound in sequence — fixing one surfaces the next — which is why they went unnoticed until traced end-to-end against a live pipeline run.

Test plan

  • Verified schema discovery returns the full column set (including day) against the live Shopify API
  • Verified replication_key/primary_keys resolve correctly post-construction
  • Verified discovered schema types the replication key as a timestamp
  • Ran full meltano run tap-shopify target-bigquery end-to-end; sales_over_time extracted and loaded correctly with no duplicate rows and correct incremental bookmarking

🤖 Generated with Claude Code

The sales_over_time stream (and any other config-driven ShopifyQLStream)
was silently broken end-to-end:

- schema discovery and row parsing both read
  data.get("shopifyqlQuery") instead of unwrapping the standard GraphQL
  "data" envelope, so every ShopifyQL response resolved to an empty
  result with no error raised.
- primary_keys/replication_key were assigned before super().__init__(),
  which unconditionally resets the backing _primary_keys/_replication_key
  attributes, silently wiping the configured values.
- the discovered schema typed every column (including the replication
  key) as a plain string, which fails singer_sdk's timestamp-based
  bookmark logic.
- ShopifyQL returns date-only strings (e.g. "2026-07-03") for TIMESERIES
  columns; normalize these to full timestamps in post_process so they
  satisfy both the declared schema type and downstream TIMESTAMP-typed
  destination columns.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant