From: | "Joel Jacobson" <joel(at)compiler(dot)org> |
---|---|
To: | "jian he" <jian(dot)universality(at)gmail(dot)com> |
Cc: | "Tomas Vondra" <tomas(dot)vondra(at)enterprisedb(dot)com>, "Tom Dunstan" <pgsql(at)tomd(dot)cc>, "Andrew Dunstan" <andrew(at)dunslane(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Do we want a hashset type? |
Date: | 2023-06-16 00:27:33 |
Message-ID: | 6cbcf89e-b603-4c8a-9214-90c2a48713d7@app.fastmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jun 15, 2023, at 11:44, jian he wrote:
> In hashset/test/sql/order.sql, can we add the following to test whether
> the optimizer will use our index.
>
> CREATE INDEX ON test_int4hashset_order (int4hashset_col
> int4hashset_btree_ops);
>
> -- to make sure that this work with just two rows
> SET enable_seqscan TO off;
>
> explain(costs off) SELECT * FROM test_int4hashset_order WHERE
> int4hashset_col = '{1,2}'::int4hashset;
> reset enable_seqscan;
Not sure I can see the value of that test,
since we've already tested the comparison functions,
which are used by the int4hashset_btree_ops operator class.
I think a test that verifies the btree index is actually used,
would more be a test of the query planner than hashset.
I might be missing something here, please tell me if so.
> Since most contrib modules, one module, only one test file, maybe we
> need to consolidate all the test sql files to one sql file
> (int4hashset.sql)?
I've also made the same observation; I wonder if it's by design
or by coincidence? I think multiple test files improves modularity,
isolation and overall organisation of the testing.
As long as we are developing in the pre-release phase,
I think it's beneficial and affordable with rigorous testing.
However, if hashset would ever be considered
for core inclusion, then we should consolidate all tests into
one file and retain only essential tests, thereby minimizing
impact on PostgreSQL's overall test suite runtime
where every millisecond matters.
> Attached is a patch slightly modified README.md. feel free to change,
> since i am not native english speaker...
>
> Attachments:
> * 0001-add-instruction-using-PG_CONFIG-to-install-extension.patch
Thanks, improvements incorporated with some minor changes.
/Joel
Attachment | Content-Type | Size |
---|---|---|
hashset-0.0.1-61d572a.patch | application/octet-stream | 1.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2023-06-16 01:25:13 | Re: Fix a typo in rewriteHandler.c |
Previous Message | Nathan Bossart | 2023-06-16 00:09:59 | Re: add non-option reordering to in-tree getopt_long |