Accept multiple --id options

[allenap]

It's useful to be able to get a database slice for multiple identifiers in a single pass.

[allenap]

This is a breaking change because custom copy-out queries will need to change conditions from blah = :id to blah in :ids. How do we want to handle this? Ordinarily this would be a major version bump, but we're pre-1.0 so I assume we release with a bump to the minor version.

Breaking change

[jelder]

@allenap Great point. I wonder if we could just run each user-defined query once for each :id without breaking anything, or making existing feats impossible.

[allenap]

I wonder if we could just run each user-defined query once for each :id without breaking anything, or making existing feats impossible.

This will break for a query like:

select * from example_table
 where id_column = :id
    or id_column is null

because we'll fetch those id_column is null rows multiple times.

In custom copy-out queries we could instead replace :id with ANY (ARRAY[...the ids...]), so that the query above expands to:

select * from example_table
 where id_column = ANY (ARRAY[...])
    or id_column is null

I've read that PostgreSQL treats IN (...) and = ANY (array) the same, i.e. one de-sugars to the other.

Queries like:

select * from example_table
 where id_column in (:id, 'some-fixed-value')

will not work, but they won't work with the :ids expansion in this branch either.

I'll try it out.

[allenap]

I tried out id_column = ANY (ARRAY[...]) and it breaks for an unexpected reason: there's no type coercion with the = ANY (ARRAY[...]) form:

I'll try adding that cast.

[allenap]

@jelder I've added a cast based on the column's udt_name and it works... but custom copy-out queries still need to be updated. For example, we expand the following:

select * from example_table
 where uuid_column = :id

into:

select * from example_table
 where uuid_column = any (array[...])

and PostgreSQL complains "operator does not exist: uuid = text" about that.

This is a confusing error message. I think we're better off sticking with the original breaking change. What do you think?

[JQuezada0]

@allenap I'm somewhat confused, after your change to cast the array it works? or it still fails?

[allenap]

@allenap I'm somewhat confused, after your change to cast the array it works? or it still fails?

It works with the explicit type casting that I added, e.g.:

select * from example_table
 where uuid_column = any (array[...] :: uuid[])

but it feels overcomplicated. I think it's better to take the incompatibility hit of changing :id to :ids than to go with this.

There's another way:

@allenap Great point. I wonder if we could just run each user-defined query once for each :id without breaking anything, or making existing feats impossible.

We could use union, e.g. given the query:

select * from example_table
 where column = :id

and the IDs "foo" and "bar", we would generate the following query:

select * from example_table
 where column = 'foo'
union
select * from example_table
 where column = 'bar'

Upside: simple, it works (probably).
Downsides:

• Lots of IDs means huge queries.
• union means that PostgreSQL has to deduplicate rows which could be expensive. We can't use union all to get around this because it may break uniqueness constraints when loading.

My conclusion remains as before: switch from :id to :ids and take the one-time hit.

[JQuezada0]

I agree w/ @allenap, idCol IN (...) seems like the simplest path, even though that'll cause a breaking change with earlier versions

[JQuezada0]

What do you think @jelder ?

[jelder]

It's a breaking change, so we need to reflect that in the version when we release it.

Also, the animated gif would have to be regenerated (it's in code).

Could we continue to support --id (singular, but used >0 times)?

[allenap]

It's a breaking change, so we need to reflect that in the version when we release it.

👍

Also, the animated gif would have to be regenerated (it's in code).

Good point. I will do it as a separate commit/merge to main, so that it shows the correct release version information.

Could we continue to support --id (singular, but used >0 times)?

Yes; this PR does that.

[allenap]

Updated to use IN :ids and tested with an example database and two --id flags. Seems to work fine.