Adds test for COPY FROM failure in Citus foreign tables

[Naisila Puka (naisila)]

No description provided.

[Önder Kalacı (onderkalaci)]

hmm, is this error coming from PG or Citus? Shouldn't we give a precise error message in EnsureCopyCanRunOnRelation? Like : Citus cannot COPY into foreign tables or such?

[Naisila Puka (naisila)]

It is a Postgres error coming from the worker nodes.

case XACT_EVENT_PRE_PREPARE:

/*
 * We disallow any remote transactions, since it's not
 * very reasonable to hold them open until the prepared
 * transaction is committed.  For the moment, throw error
 * unconditionally; later we might allow read-only cases.
 * Note that the error will cause us to come right back
 * here with event == XACT_EVENT_ABORT, so we'll clean up
 * the connection state at that point.
 */
ereport(ERROR,
		(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
		 errmsg("cannot PREPARE a transaction that has operated on postgres_fdw foreign tables")));
break;

I agree we should catch this error earlier and throw a more meaningful message. Given that it is out of PG16 scope, I will open an issue about it. Either way, I think it makes sense to keep this in our tests for now so that we are aware COPY FROM doesn't work for citus foreign tables.

[Önder Kalacı (onderkalaci)]

I think I'd fix it right now, because otherwise we always have to keep in mind that on the shell table we allow something that Postgres doesn't allow. We might already have some edge cases, like what if the COPY is on a local shard and we do not use prepared transactions?

The fix should be trivial anyway?

[Naisila Puka (naisila)]

I will open the issue and prioritize it for this week.
#7176

[Marco Slot (marcocitus)]

The error seems to ok to me (it correctly assigns blame to postgres_fdw), but maybe we should make COPY not initiate a 2PC if it only touches a single shard? That way, we avoid this error in many cases.

[Önder Kalacı (onderkalaci)]

The error seems to ok to me

after a second look, I think the error message is acceptable, but still I prefer to throw error earlier to prevent edge cases (e.g., Citus' some code-paths might fail for foreign tables and causes crash/worse error messages
as we allowed the command pass through etc)

but maybe we should make COPY not initiate a 2PC if it only touches a single shard? That way, we avoid this error in many cases.

maybe we change 7176 to document such improvement idea