Select used instead of Perform in migration causes issues when executing the migration SQL via PSQL

[nauticalcoder]

The migration that is created to convert the existing Serial columns to Identity fails to run if the SQL is executed via the psql command line which is how we deploy our database migrations via Bamboo. The error is because a SELECT is used without using the returned value. Here is an example:

DO $$
BEGIN
IF NOT EXISTS(SELECT 1 FROM "__EFMigrationsHistory" WHERE "MigrationId" = '20191018172133_AddWorldCities') THEN
ALTER TABLE "VisaStatuses" ALTER COLUMN "Id" TYPE integer;
ALTER TABLE "VisaStatuses" ALTER COLUMN "Id" SET NOT NULL;
ALTER SEQUENCE "VisaStatuses_Id_seq" RENAME TO "VisaStatuses_Id_old_seq";
ALTER TABLE "VisaStatuses" ALTER COLUMN "Id" DROP DEFAULT;
ALTER TABLE "VisaStatuses" ALTER COLUMN "Id" ADD GENERATED BY DEFAULT AS IDENTITY;
SELECT * FROM setval('"VisaStatuses_Id_seq"', nextval('"VisaStatuses_Id_old_seq"'), false);
DROP SEQUENCE "VisaStatuses_Id_old_seq";
END IF;
END $$;

psql:know-more-domain-migrations.sql:3605: ERROR:  query has no destination for result data
HINT:  If you want to discard the results of a SELECT, use PERFORM instead.
CONTEXT:  PL/pgSQL function inline_code_block line 9 at SQL statement

I believe simply changing the generated SQL to use a PERFORM instead of a SELECT for this line:
SELECT * FROM setval('"VisaStatuses_Id_seq"', nextval('"VisaStatuses_Id_old_seq"'), false);
would solve the problem.

[austindrenski]

Reproduced:

=> CREATE SEQUENCE test_seq;
=> SELECT * FROM setval('test_seq', nextval('test_seq'), false);
 setval
--------
      1
(1 row)

=> DO $$
$> BEGIN
$> SELECT * FROM setval('test_seq', nextval('test_seq'), false);
$> END $$;
ERROR:  query has no destination for result data
HINT:  If you want to discard the results of a SELECT, use PERFORM instead.
CONTEXT:  PL/pgSQL function inline_code_block line 3 at SQL statement

=> DO $$
$> BEGIN
$> PERFORM setval('test_seq', nextval('test_seq'), false);
$> END $$;
DO

[roji]

Weird, I was sure we had a test for this... We should add one...

[roji]

Oh actually these migration scenarios are usually not actually tested on a real database, just asserts on the SQL - which is why an error like this can slip through...

Anyway, looks good to me, let's fix it.

[roji]

Unfortunately this broke normal migrations, since PERFORM only works in PL/pgSQL (DO blocks), but not in regular SQL.

Idempotent migrations are a more rare feature, and in any case they can be edited to replace PERFORM with SELECT (whereas normal migrations are applied directly by the application and can't be modified). So I'm unfortunately reverting this for 3.1.1 (via #1156), and have opened #1157 to track fixing this in a correct way for all migration types.

@nauticalcoder can you confirm whether you're running idempotent scripts, or was it some other scenario where DO blocks are being used?

[nauticalcoder]

We are currently using idempotent scripts. This is the command we are using to generate the migration script which is then executed in Bamboo with psql.

dotnet tool run dotnet-ef \
  --configuration Release \
  migrations \
  script \
  --idempotent \
  --output \
  ../../artifacts/project-domain-migrations.sql

[roji]

@nauticalcoder thanks for confirming - makes sense.

Unfortunately, the way things stand at the moment, we can only make one scenario work - either idempotent or non-idempotent - and as I wrote above we're going to have to allow non-idempotent migrations.

On the other hand, note that the offending function call (setval) should only be generated once, when you transition your database from SERIAL-based columns to IDENTITY-based ones. That's typically a single migration in the lifetime of your project, which you can patch manually in the idempotent SQL script.

[nauticalcoder]

Yeah, we don't anticipate having it pop up again as all of our environments have now been upgraded. We ended up running the migrations manually as you stated to get around the issue. Thanks for taking the time to look into it.

[roji]

Sure thing!

[roji]

Note that this is being fixed again in #1479 for idempotent migrations (but without affecting non-idempotent ones)