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

Add support for hstore #6

Closed
wants to merge 3 commits into from
Closed

Add support for hstore #6

wants to merge 3 commits into from

Conversation

bullno1
Copy link
Contributor

@bullno1 bullno1 commented Feb 20, 2014

Currently typespec is:

hstore() :: [{key(), value()}] | {[key(), value()}.
key() :: atom() | binary() | string() | number().
value() :: null | atom() | binary() | string() | number().

"jiffy-like" format is required for array support since there's no trivial way to distinguish between an array of hstore and a hstore.
Return type for hstore is always: [{binary(), binary() | null}] for convenience. Maps in 17.0 will solve this problem though.

Opened a pull request because I need some discussion about this.

@seriyps
Copy link
Member

seriyps commented Feb 20, 2014

Maybe, as you say, define hstore strictly on jiffy-like format {[ {key(), value()} ]} and drop [ {key(), value()} ]? Including return value. This should fix any ambiguity.

@davidw
Copy link
Member

davidw commented Feb 28, 2014

I would vote for avoiding ambiguity.

@bullno1
Copy link
Contributor Author

bullno1 commented Mar 15, 2014

Would 17.0's map be better?
I don't have any urgent need for it right now anyway.

@davidw
Copy link
Member

davidw commented Mar 15, 2014

Given that R17 isn't even released yet, and the conservative nature of many in the Erlang community, I think a solution that only works for that is not optimal. I think I would try and do something like:

  • A reasonable solution for older Erlangs. Essential.
  • A map-based solution for new Erlangs. Nice to have.

@bullno1
Copy link
Contributor Author

bullno1 commented Apr 11, 2014

jiffy-like format is enforced now

@davidw
Copy link
Member

davidw commented Apr 14, 2014

This has been merged into the devel branch! Thank you!

@davidw davidw closed this Apr 14, 2014
@davidw
Copy link
Member

davidw commented Apr 14, 2014

I'm actually getting some errors with the tests. What version are you running of Postgres?

@davidw davidw reopened this Apr 14, 2014
@davidw
Copy link
Member

davidw commented Apr 14, 2014

   pgsql_tests: all_test_ (hstore_type_test(pgsql))...*failed*
in function pgsql_tests:'-check_type/6-fun-2-'/7 (test/pgsql_tests.erl, line 829)
in call from pgsql_tests:with_connection/4 (test/pgsql_tests.erl, line 788)
in call from pgsql_tests:hstore_type_test/1 (test/pgsql_tests.erl, line 557)
**error:{badmatch,{ok,[{column,<<"hstore">>,{unknown_oid,255845},-1,-1,0}],[{<<>>}]}}

@bullno1
Copy link
Contributor Author

bullno1 commented Apr 14, 2014

Did you update the test schema?

It needs "CREATE EXTENSION hstore" and an extra column to do the serialization test.

bullno1@7e37aff#diff-b89470b98ce374506061ea6587f445edR38

@davidw
Copy link
Member

davidw commented Apr 14, 2014

Yes, I did. I'm on Postgres 9.1.13

@bullno1
Copy link
Contributor Author

bullno1 commented Apr 18, 2014

My make test result:

Failed: 6.  Skipped: 0.  Passed: 51.

As usual, those are ssl tests.
I'm running postgres 9.3.4-1 on ArchLinux. Let me get a VM to test 9.1.13.
Still, why would they change oid between versions?

@davidw
Copy link
Member

davidw commented Apr 18, 2014

SELECT n.nspname as "Schema",
  t.oid,
  pg_catalog.format_type(t.oid, NULL) AS "Name",
  pg_catalog.obj_description(t.oid, 'pg_type') as "Description"
FROM pg_catalog.pg_type t
     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace
WHERE (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid))
  AND NOT EXISTS(SELECT 1 FROM pg_catalog.pg_type el WHERE el.oid = t.typelem AND el.typarray = t.oid)
  AND pg_catalog.pg_type_is_visible(t.oid)
ORDER BY 1, 2;

Shows this:

public     | 256714 | hstore                      | 
public     | 256793 | ghstore                     |

@bullno1
Copy link
Contributor Author

bullno1 commented Apr 18, 2014

Yes, it's totally random:

SELECT typname, typelem, typarray FROM pg_type WHERE typname LIKE '%hstore%';

shows

 typname  | typelem | typarray 
----------+---------+----------
 hstore   |       0 |    16672
 _hstore  |   16667 |        0
 ghstore  |       0 |    16749
 _ghstore |   16746 |        0

@davidw
Copy link
Member

davidw commented Apr 18, 2014

Idea: API to fill in type cache (ets or the process dictionary?) by querying the database, that is called at connect time, and whenever the user wants. So, if you know you are going to fool around with extensions during your session, you can regenerate the type cache.

@davidw
Copy link
Member

davidw commented Apr 22, 2014

I have pushed a version of this to the dynamic_types branch - all the tests pass, so have a look at it and see what you think.

@davidw
Copy link
Member

davidw commented Jan 28, 2015

We have hstore support now. Thanks!

@davidw davidw closed this Jan 28, 2015
keynslug added a commit to keynslug/epgsql that referenced this pull request Jan 8, 2023
chore: merge 'upstream/devel' into master
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.

None yet

3 participants