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

unsupported comparison operator: <oid> NOT IN <tuple{int}> #14554

Closed
tlvenn opened this issue Apr 3, 2017 · 24 comments · Fixed by #37578
Closed

unsupported comparison operator: <oid> NOT IN <tuple{int}> #14554

tlvenn opened this issue Apr 3, 2017 · 24 comments · Fixed by #37578
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community
Milestone

Comments

@tlvenn
Copy link
Contributor

tlvenn commented Apr 3, 2017

As discussed with @jordanlewis in #12526, I believe the recent work on supporting the OID type created a small regression with the following error:

sql/executor.go:722 [client=127.0.0.1:63505,user=root,n1] execRequest: error: unsupported comparison operator: <oid> NOT IN <tuple{int}>

Looks like there are some missing comparison operators between OID and int type now that OIDs are not int anymore.

@jordanlewis
Copy link
Member

@tlvenn, could you please provide the full query that's being issued by Postgrex here? I've looked at the code that generates it, but I'm afraid I'm not fluent enough in Elixir to be able to construct the query by staring at the code.

@jordanlewis
Copy link
Member

The underlying issue here is the following query:

SELECT ... WHERE t.oid NOT IN
  (SELECT (ARRAY[integer, literals, ...])[i]
   FROM generate_series(1, length_of_above_array))

The reason it fails to type properly is that our type system currently does not correctly propagate desired types into subqueries. If that was done correctly, the query would type properly, as the type system knows that t.oid is an oid type and would propagate that information all the way into the array literal in the subquery, allowing the integer literals to be automatically upcasted to oids.

Fixing this issue can be done (at least) two ways:

  • Improve subquery typing to correctly propagate desired types into subqueries.
  • Add integer-oid comparison builtins. This is more of a bandaid. We've managed to avoid needing to do this so far, and it'd be nice to continue avoiding this as our type system should be able to handle such comparisons in the majority of cases without adding special-case comparison logic.

As an alternative, I've opened elixir-ecto/postgrex#310 since it appears that this subquery is not necessary and can be replaced by a list of integer literals.

@jordanlewis
Copy link
Member

For the record, the full query getting executed by Postgrex is:

SELECT t.oid, t.typname, t.typsend, t.typreceive, t.typoutput, t.typinput,
	       t.typelem, coalesce(r.rngsubtype, 0), ARRAY (
	  SELECT a.atttypid
	  FROM pg_attribute AS a
	  WHERE a.attrelid = t.typrelid AND a.attnum > 0 AND NOT a.attisdropped
	  ORDER BY a.attnum
	)
	FROM pg_type AS t
	LEFT JOIN pg_range AS r ON r.rngtypid = t.oid
	WHERE t.oid NOT IN (
	  SELECT (ARRAY[704,11676,10005,3912,11765,59410,11397,12416,1266,1001,705,199,12537,12472,11705,3907,1017,12574,11395,1019,25,12397,12525,12450,11640,11396,603,12394,1012,604,11802,12540,11648,59175,11638,11616,1186,1082,11655,11631,3927,11647,3770,12551,11844,11733,11692,651,12601,59197,11719,1043,12402,11788,11785,10157,11688,11672,11822,12593,11626,12532,12431,12578,2204,1009,12487,3831,21,650,12642,24,59194,2206,325,2202,1015,210,11782,10004,12616,12454,1021,1005,11792,3807,11653,1002,1013,1018,2209,11646,11641,83,59323,12522,12515,11635,2210,11622,26,11840,10156,12409,1563,11795,11628,2278,1025,20,12428,12420,628,11702,10003,1022,1028,143,12442,11809,11642,1700,12462,1270,2949,602,11649,59193,12494,12639,11715,10002,1042,11806,1010,3910,11402,1006,2280,2951,12635,12512,11727,11637,791,719,12396,59177,2281,12491,12649,12563,11724,11712,12476,11623,11621,3310,12501,703,28,23,10006,2276,1041,3221,1185,11633,11662,11819,12629,11836,3769,16,11778,11636,11625,12510,3838,3908,12399,1023,12622,11730,11816,11651,4096,11624,2950,600,10000,81,75,12458,3913,2249,1007,12632,59181,12605,2843,1033,12520,12438,11629,11399,11400,1008,11812,11772,11617,3500,4089,12434,2279,12405,11401,3220,718,32,12613,1248,10846,3643,11684,2201,790,12505,11833,829,59328,12548,2970,1014,3615,11394,12559,2205,12517,4090,12609,11630,3644,3734,12571,1027,19,3735,1231,2287,3645,1083,12544,11762,18,12645,11798,29,22,114,11398,2207,27,12585,11736,3911,11830,1114,2282,1561,1184,12619,11699,2277,1562,1003,59407,12480,12465,12424,59188,12401,12446,11668,3906,3905,3614,11639,1011,12535,11775,1790,1263,11826,12507,71,4066,3642,17,12555,11680,11645,59196,11696,11644,3909,59180,2842,629,12498,59174,11632,3115,59189,59182,1560,1182,1034,11740,12653,11650,194,2275,2208,12581,11755,12597,11768,702,11627,11393,1115,1183,11748,11659,10001,11744,3926,1000,1187,3802,3904,11708,2776,1040,11652,11620,12567,12589,11665,11619,59183,701,11643,4097,11758,2211,30,12469,12412,1020,11618,2203,12625,12484,12527,11634,12530,2283,869,11751,1024,1016,601,142,700])[i]
	  FROM generate_series(1, 376) AS i
	)

@tlvenn
Copy link
Contributor Author

tlvenn commented Apr 6, 2017

Hi @jordanlewis, sorry was pretty busy the past 2 days and I am only now catching up with emails, github notifications and the like.

Unfortunately, we can't use yet the default Postgrex driver because the following query still dont run in CDB:

SELECT t.oid, t.typname, t.typsend, t.typreceive, t.typoutput, t.typinput,
	       t.typelem, coalesce(r.rngsubtype, 0), ARRAY (
	  SELECT a.atttypid
	  FROM pg_attribute AS a
	  WHERE a.attrelid = t.typrelid AND a.attnum > 0 AND NOT a.attisdropped
	  ORDER BY a.attnum
	)
	FROM pg_type AS t
	LEFT JOIN pg_range AS r ON r.rngtypid = t.oid

So I am still maintaining a fork at https://github.com/jumpn/postgrex that is getting rid of that ARRAY and was remapping types properly. I was however still using the filter OID part which started to fail once you merged your work on OID type, and that is how I stumbled on it.

For now I am doing an explicit cast to int as a workaround.

@jordanlewis
Copy link
Member

Indeed, I didn't notice because of the other issues above, but this query uses a correlated subquery which we don't support yet - see #3288.

@tlvenn
Copy link
Contributor Author

tlvenn commented Apr 7, 2017

Ha thanks for the pointer @jordanlewis, looks like I will have to maintain my fork for a little bit longer ;)

@jordanlewis
Copy link
Member

Unfortunately it looks that way, yes. Thank you for sticking with it - really appreciate your persistence on this issue! 😊

@dianasaur323 dianasaur323 added this to the Later milestone Apr 19, 2017
@bdarnell
Copy link
Contributor

The java SQL library jOOQ also needs this feature according to this tweet

@jordanlewis
Copy link
Member

The query in question that jOOQ generates is as follows:

select 
  "pg_catalog"."pg_namespace"."nspname", 
  "pg_catalog"."pg_type"."typname"
from "pg_catalog"."pg_type"
  join "pg_catalog"."pg_namespace"
  on "pg_catalog"."pg_type"."typnamespace" = "pg_catalog"."pg_namespace".oid
where (
  "pg_catalog"."pg_namespace"."nspname" in ('bank')
  and "pg_catalog"."pg_type".oid in (
    select "pg_catalog"."pg_enum"."enumtypid"
    from "pg_catalog"."pg_enum"
  )
)
order by 
  "pg_catalog"."pg_namespace"."nspname" asc, 
  "pg_catalog"."pg_type"."typname" asc

@ghost
Copy link

ghost commented Aug 23, 2017

DataNucleus uses a table validation strategy during table creation/generation which also uses this comparison.

Full query:

SELECT n.nspname = ANY(current_schemas(TRUE)),
       n.nspname,
       t.typname
FROM pg_catalog.pg_type t
JOIN pg_catalog.pg_namespace n ON t.typnamespace = n.oid
WHERE t.oid = $1

Using version CockroachDB v1.1-alpha.20170817

@knz
Copy link
Contributor

knz commented Aug 23, 2017

@jordanlewis could you have a look at the last query pasted above from @Fathardie? It seems to me that this is a simpler case for which you already have done some thinking (namely, ensure that an OID placeholder delivered by the client as int during execute is treated as OID during the 2nd type checking).

@knz
Copy link
Contributor

knz commented Aug 23, 2017

If this is indeed a separate case, please fork that sub-discussion into a separate issue (or reuse existing issue if there's one)

@jordanlewis
Copy link
Member

This will be solved by #16672 and is the same as #14311.

@jordanlewis jordanlewis added C-question A question rather than an issue. No code/spec/doc change needed. O-community Originated from the community and removed O-deprecated-community-questions labels Apr 24, 2018
@tlvenn
Copy link
Contributor Author

tlvenn commented Apr 29, 2019

Hi @jordanlewis,

Now that CDB with the latest v19 version is handling correlated subqueries , I though I would test what is happening now with Postgrex, given it should pretty much work or so I hoped ;)

** (Postgrex.Error) ERROR 22023 (invalid_parameter_value): unsupported comparison operator: <oid> IN <tuple{int}>
    (db_connection) lib/db_connection/connection.ex:163: DBConnection.Connection.connect/2
    (connection) lib/connection.ex:622: Connection.enter_connect/5
    (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

Wasn't this supposed to be fixed with #16672 ? Any idea why I am still running into this ?
Thanks in advance !

For info, this is with:

Build Tag:    v19.1.0-rc.4
Build Time:   2019/04/24 13:51:46

@jordanlewis
Copy link
Member

Hey @tlvenn, glad you're waiting with bated breath, haha - and hmm yeah I would have expected this to not come up anymore. Could you send me server logs generated with --vmodule=conn_executor=2 when you try to connect, please?

@tlvenn
Copy link
Contributor Author

tlvenn commented Apr 30, 2019

here you go:

I190430 00:03:28.056018 698 sql/conn_executor.go:1155  [n1,client=127.0.0.1:52797,user=root] [NoTxn pos:0] executing ExecStmt: SELECT t.oid, t.typname, t.typsend, t.typreceive, t.typoutput, t.typinput, COALESCE(d.typelem, t.typelem), COALESCE(r.rngsubtype, 0), ARRAY (SELECT a.atttypid FROM pg_attribute AS a WHERE ((a.attrelid = t.typrelid) AND (a.attnum > 0)) AND (NOT a.attisdropped) ORDER BY a.attnum) FROM pg_type AS t LEFT JOIN pg_type AS d ON t.typbasetype = d.oid LEFT JOIN pg_range AS r ON (r.rngtypid = t.oid) OR ((t.typbasetype != 0) AND (r.rngtypid = t.typbasetype)) WHERE t.oid NOT IN (SELECT (ARRAY[1001, 25, 1082, 1186, 1043, 1009, 21, 24, 2206, 1015, 2202, 1005, 1021, 1002, 26, 1563, 20, 1022, 1028, 1700, 1042, 2951, 23, 1041, 1185, 16, 2950, 1007, 2249, 4089, 1014, 2205, 19, 1231, 1083, 18, 22, 1114, 1184, 1561, 2277, 1562, 1003, 17, 1560, 1182, 1115, 1183, 1000, 1187, 3802, 701, 30, 2283, 869, 1016, 700])[i] FROM ROWS FROM (generate_series(1, 57)) AS i)
I190430 00:03:28.056073 698 sql/conn_executor.go:1155  [n1,client=127.0.0.1:52797,user=root] [Open pos:0] executing ExecStmt: SELECT t.oid, t.typname, t.typsend, t.typreceive, t.typoutput, t.typinput, COALESCE(d.typelem, t.typelem), COALESCE(r.rngsubtype, 0), ARRAY (SELECT a.atttypid FROM pg_attribute AS a WHERE ((a.attrelid = t.typrelid) AND (a.attnum > 0)) AND (NOT a.attisdropped) ORDER BY a.attnum) FROM pg_type AS t LEFT JOIN pg_type AS d ON t.typbasetype = d.oid LEFT JOIN pg_range AS r ON (r.rngtypid = t.oid) OR ((t.typbasetype != 0) AND (r.rngtypid = t.typbasetype)) WHERE t.oid NOT IN (SELECT (ARRAY[1001, 25, 1082, 1186, 1043, 1009, 21, 24, 2206, 1015, 2202, 1005, 1021, 1002, 26, 1563, 20, 1022, 1028, 1700, 1042, 2951, 23, 1041, 1185, 16, 2950, 1007, 2249, 4089, 1014, 2205, 19, 1231,1083, 18, 22, 1114, 1184, 1561, 2277, 1562, 1003, 17, 1560, 1182, 1115, 1183, 1000, 1187, 3802, 701, 30, 2283, 869, 1016, 700])[i] FROM ROWS FROM (generate_series(1, 57)) AS i)
I190430 00:03:28.056527 698 sql/conn_executor.go:1331  [n1,client=127.0.0.1:52797,user=root] execution error: unsupported comparison operator: <oid> IN <tuple{int}>

@jordanlewis
Copy link
Member

Sigh, looks like this didn't really get closed by the linked issue. That was about making sure placeholders got typed correctly in similar circumstances. We still don't support mixed type comparisons of this nature. I'll see about making this happen sooner rather than later.

@jordanlewis jordanlewis reopened this May 1, 2019
@tlvenn
Copy link
Contributor Author

tlvenn commented May 1, 2019

Thanks @jordanlewis that would be awesome, i believe this is the final bit to be able to use Elixir Postgresql driver without any fork whatsoever.

@jordanlewis
Copy link
Member

Hey @tlvenn, as much as I don't like to admit it, fixing this isn't easy. The change to the query, on the other hand, is very easy and will be compatible in both PG and CRDB. Would you consider it? All you would have to do is add a single ::int cast to t.oid - so you'd have t.oid::INT NOT IN (...).

tlvenn added a commit to tlvenn/postgrex that referenced this issue May 3, 2019
The latest version of CockroachDB comes with full support for correlated subqueries which was an issue before. Now the only stoper is one sql fragment where an explicity cast is currently needed.

See: cockroachdb/cockroach#14554
@tlvenn
Copy link
Contributor Author

tlvenn commented May 3, 2019

elixir-ecto/postgrex#460

Crossing fingers ;)

josevalim pushed a commit to elixir-ecto/postgrex that referenced this issue May 5, 2019
The latest version of CockroachDB comes with full support for correlated subqueries which was an issue before. Now the only stoper is one sql fragment where an explicity cast is currently needed.

See: cockroachdb/cockroach#14554
@tlvenn
Copy link
Contributor Author

tlvenn commented May 8, 2019

Change has been merged on Postgrex side so all is good now as far as Elixir is concerned 🎉

@dianasaur323 you might want to update the labels as I believe this should be classified as a bug or an enhancement.

@jordanlewis
Copy link
Member

@tlvenn this is awesome! So happy to hear it - it's been a bit of a saga.

As a small thank you for sticking with us, if you want, email me at jordan@cockroachlabs.com with your tshirt size and mailing address and we'll send you a small gift :)

@jordanlewis jordanlewis added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) and removed C-question A question rather than an issue. No code/spec/doc change needed. labels May 8, 2019
@tlvenn
Copy link
Contributor Author

tlvenn commented May 9, 2019

Thanks will do ! Btw did you mean to close the issue ?

@jordanlewis
Copy link
Member

Heh, nope.

@jordanlewis jordanlewis reopened this May 9, 2019
rytaft added a commit to rytaft/cockroach that referenced this issue May 17, 2019
Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until TypeCheck is called for
   the first time.

2. For subqueries on the right side of an IN expression, the desired type
   passed into TypeCheck was AnyTuple. This resulted in an error later on in
   typeCheckSubqueryWithIn, which checks to make sure the type of the subquery
   is tuple{T} where T is the type of the left expression. Now the desired
   type passed into TypeCheck is tuple{T}.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes cockroachdb#37263
Fixes cockroachdb#14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.
rytaft added a commit to rytaft/cockroach that referenced this issue May 20, 2019
Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until TypeCheck is called for
   the first time.

2. For subqueries on the right side of an IN expression, the desired type
   passed into TypeCheck was AnyTuple. This resulted in an error later on in
   typeCheckSubqueryWithIn, which checks to make sure the type of the subquery
   is tuple{T} where T is the type of the left expression. Now the desired
   type passed into TypeCheck is tuple{T}.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes cockroachdb#37263
Fixes cockroachdb#14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.
craig bot pushed a commit that referenced this issue May 20, 2019
37506: storage: reconcile manual splitting with automatic merging r=jeffrey-xiao a=jeffrey-xiao

Follows the steps outlined in #37487 to reconcile manual splitting with automatic merging. This PR includes the changes needed at the storage layer. The sticky bit indicating that a range is manually split is added to the range descriptor.

37558: docs/tla-plus: add timestamp refreshes to ParallelCommits spec r=nvanbenschoten a=nvanbenschoten

This commit adds transaction timestamp refreshes to the PlusCal model
for parallel commits. This allows the committing transaction to bump
its timestamp without a required epoch bump.

This completes the parallel commits formal specification.

37578: opt, sql: fix type inference of TypeCheck for subqueries r=rytaft a=rytaft

Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until `TypeCheck` is called for
   the first time.

2. For subqueries on the right side of an `IN` expression, the desired type
   passed into `TypeCheck` was `AnyTuple`. This resulted in an error later on in
   `typeCheckSubqueryWithIn`, which checks to make sure the type of the subquery
   is `tuple{T}` where `T` is the type of the left expression. Now the desired
   type passed into `TypeCheck` is `tuple{T}`.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes #37263
Fixes #14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.

Co-authored-by: Jeffrey Xiao <jeffrey.xiao1998@gmail.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
@craig craig bot closed this as completed in #37578 May 20, 2019
rytaft added a commit to rytaft/cockroach that referenced this issue May 20, 2019
Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until TypeCheck is called for
   the first time.

2. For subqueries on the right side of an IN expression, the desired type
   passed into TypeCheck was FamTuple. This resulted in an error later on in
   typeCheckSubqueryWithIn, which checks to make sure the type of the subquery
   is tuple{T} where T is the type of the left expression. Now the desired
   type passed into TypeCheck is tuple{T}.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes cockroachdb#37263
Fixes cockroachdb#14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.
tolbrino pushed a commit to tolbrino/postgrex that referenced this issue May 27, 2019
The latest version of CockroachDB comes with full support for correlated subqueries which was an issue before. Now the only stoper is one sql fragment where an explicity cast is currently needed.

See: cockroachdb/cockroach#14554
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants