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

Restore datetime functions in SQL #4898

Closed
tsafin opened this issue Apr 16, 2020 · 5 comments
Closed

Restore datetime functions in SQL #4898

tsafin opened this issue Apr 16, 2020 · 5 comments
Labels
feature A new functionality performance sql

Comments

@tsafin
Copy link
Contributor

tsafin commented Apr 16, 2020

15 out of 22 TPC-H queries use some sort of datetime functions, .e.g. q1.sql

-- using 1433771997 as a seed to the RNG


select
	l_returnflag,
	l_linestatus,
	sum(l_quantity) as sum_qty,
	sum(l_extendedprice) as sum_base_price,
	sum(l_extendedprice * (1 - l_discount)) as sum_disc_price,
	sum(l_extendedprice * (1 - l_discount) * (1 + l_tax)) as sum_charge,
	avg(l_quantity) as avg_qty,
	avg(l_extendedprice) as avg_price,
	avg(l_discount) as avg_disc,
	count(*) as count_order
from
	lineitem
where
	l_shipdate <= date('1998-12-01', '-71 days')
group by
	l_returnflag,
	l_linestatus
order by
	l_returnflag,
	l_linestatus;

at the moment it's disabled in src/sq/date.c

WE need to restore them, becase they block #2903 TPC-H benchmarks.

@kyukhin kyukhin added the feature A new functionality label Apr 17, 2020
@pgulutzan
Copy link
Contributor

pgulutzan commented Apr 17, 2020

That's SQLite syntax. The original is
http://www.tpc.org/tpc_documents_current_versions/pdf/tpc-h_v2.18.0.pdf

select
       l_returnflag,
       l_linestatus,
       sum(l_quantity) as sum_qty,
       sum(l_extendedprice) as sum_base_price,
       sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
       sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
       avg(l_quantity) as avg_qty,
       avg(l_extendedprice) as avg_price,
       avg(l_discount) as avg_disc,
       count(*) as count_order
from
       lineitem
where
       l_shipdate <= date '1998-12-01' -interval '71' day
group by
       l_returnflag,
       l_linestatus
order by
       l_returnflag,
       l_linestatus;

(substituting '71' for '[DELTA]' as allowed by 2.4.1.3)
So, by restoring in order to handle q1.sql, you won't get exactly
what TPC specifies.

@tsafin tsafin self-assigned this Apr 17, 2020
@tsafin
Copy link
Contributor Author

tsafin commented Apr 17, 2020

That's SQLite syntax. The original is
http://www.tpc.org/tpc_documents_current_versions/pdf/tpc-h_v2.18.0.pdf

Yup, I know, but such modifications are allowable to be made by vendor to complete queries. In a longer run we will return vanilla syntax, but today we are interested to compare against SQLite. To measure the baseline.

If you are curios - look here https://github.com/tsafin/tnt-tpch/blob/master/execute_query.lua (do not be confused with README, I've not yet updated it)

@kyukhin kyukhin added this to the 2.5.1 milestone Apr 21, 2020
@tsafin
Copy link
Contributor Author

tsafin commented Apr 30, 2020

Waiting till tarantool/tpch#1 be resolved before transferring to QA

tsafin added a commit to tsafin/tarantool that referenced this issue Apr 30, 2020
Commit #3caeb33cdaf4b37974fd5f12e08310502d9e73f3 has disabled
DATETIME support in Tarantool SQL. Now we are restoring it
to make possible to run TPC-H queries. We restore functions
and they will use underlying STRING type, instead of actual
date types. This is better than nothing for today.

We will complete actual date/datetime types support once
DECIMAL type will land here.

Closes tarantool#4898
tsafin added a commit to tsafin/tarantool that referenced this issue Apr 30, 2020
Commit #3caeb33cdaf4b37974fd5f12e08310502d9e73f3 has disabled
all datetime tests. Now, having restored datetime functions support
we revert it, and restore sql testing for those functions

Closes tarantool#4898
@pgulutzan
Copy link
Contributor

If it is true that "in a longer run we will return vanilla syntax",
why does TPC-H have priority? The patch for SQLite functions will
not be compatible with other DBMSs, or with Lua date/time functions,
or with what we regard as good ways to do functions. For example:
(1) date('now','+10000000 days') does not return an error,
julianday('0000-04-31') does not return an error.
For number arithmetic or for some other (non-date) Tarantool/SQL functions we return errors when input or output is invalid.
(2) (Lua) os.date("%x", 906000490))=09/16/98,
(SQL) date(906000490,'unixepoch') = 1998-09-17.

@igormunkin
Copy link
Collaborator

AFAIU, the issue is resolved in scope of this commit. Closing then.

@igormunkin igormunkin closed this as not planned Won't fix, can't repro, duplicate, stale Apr 13, 2023
@kyukhin kyukhin removed this from the wishlist milestone May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new functionality performance sql
Projects
None yet
Development

No branches or pull requests

5 participants