Skip to content

Commit

Permalink
Fix wrong OID format type in repack_trigger
Browse files Browse the repository at this point in the history 
table OID should be unsigned four-byte integer, therefore formatting with %d is wrong. %u should be used as in other places.
It's surprising that we haven't received such error messages before. But such a query is definitely wrong "INSERT INTO repack.log_-907750756(pk, row) "
  • Loading branch information
@Melkij
Melkij committed Dec 14, 2023
1 parent 624745b commit ca7a175
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/repack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,12 @@ repack_trigger(PG_FUNCTION_ARGS)

/* prepare INSERT query */
sql = makeStringInfo();
appendStringInfo(sql, "INSERT INTO repack.log_%d(pk, row) "
appendStringInfo(sql, "INSERT INTO repack.log_%u(pk, row) "
"VALUES(CASE WHEN $1 IS NULL THEN NULL ELSE (ROW(", relid);
appendStringInfo(sql, "$1.%s", quote_identifier(trigdata->tg_trigger->tgargs[0]));
for (int i = 1; i < trigdata->tg_trigger->tgnargs; ++i)
appendStringInfo(sql, ", $1.%s", quote_identifier(trigdata->tg_trigger->tgargs[i]));
appendStringInfo(sql, ")::repack.pk_%d) END, $2)", relid);
appendStringInfo(sql, ")::repack.pk_%u) END, $2)", relid);

/* execute the INSERT query */
execute_with_args(SPI_OK_INSERT, sql->data, 2, argtypes, values, nulls);
Expand Down

5 comments on commit ca7a175

@bradymholt
Copy link

@bradymholt bradymholt commented on ca7a175 May 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @Melkij - I just hit this error in 1.5.0 😅 Do you know of a way I could work around it?

@Melkij
Copy link
Collaborator Author

@Melkij Melkij commented on ca7a175 May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's expected. This bug exists only in 1.5.0. Compile new one from the master branch or use an older version. Well, you can probably also redefine the trigger in plpgsql instead of C. (like older versions)

@bradymholt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I'll compile from master and use that.

@dvarrazzo
Copy link
Member

@dvarrazzo dvarrazzo commented on ca7a175 May 24, 2024 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Melkij
Copy link
Collaborator Author

@Melkij Melkij commented on ca7a175 May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the bug is important enough to make an unplanned release six months ago, because the bug makes the processed table read-only (depending on actual oid). But I was not invited to any discussions about release plans for 1.5.0 or later (for example, where it was decided that it would be 1.5.0 and not 1.4.next). So I don't know the plans for the next release.

Please sign in to comment.