Skip to content
This repository has been archived by the owner on Dec 14, 2023. It is now read-only.

test: replace add_date() calls in TPC-H queries with Ibis function calls #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ianmcook
Copy link

@ianmcook ianmcook commented Apr 12, 2023

I was working with dates in Ibis and noticed that the TPC-H queries in this repo do literal date +/- interval math using a utility function that uses Python's datetime and dateutil. I reimplemented that all here using ibis.date() +/- ibis.interval().

In 31bb61f I added a bunch of .cast("date") but I'm not sure whether they are needed. It was super unclear to me whether the date columns are stored as dates or as strings.

@ianmcook ianmcook changed the title test: replace add_date()calls in TPC-H queries with Ibis method calls test: replace add_date()calls in TPC-H queries with Ibis function calls Apr 12, 2023
@ianmcook ianmcook changed the title test: replace add_date()calls in TPC-H queries with Ibis function calls test: replace add_date() calls in TPC-H queries with Ibis function calls Apr 12, 2023
@ianmcook
Copy link
Author

I'll look at the test failures and make some changes

@gforsyth
Copy link
Member

I think you can remove the cast but keep the calls to ibis.date(DATE)

The cast would be a no-op except that the initial sqlite schema has those dates as non-nullable and we default to nullable in the casts, so you'd have to pass l_shipdate.cast('date', nullable=False) everywhere and that would be a little noisy, I think.

@ianmcook
Copy link
Author

@gforsyth should I make any changes here based on the additions in ibis-project/ibis#6003?

@gforsyth
Copy link
Member

@ianmcook -- so I think the only things we can do here are:

  1. Nothing / wait
  2. Change the sqlite CI job to run on ibis master instead of the latest release.

But that will mean that ibis-master will be a defacto requirement of running this helper with sqlite and I'm not sure we want to do that.

@ianmcook
Copy link
Author

Waiting seems fine to me. This is not urgent at all.

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

Successfully merging this pull request may close these issues.

2 participants