Skip to content

Addbdeb 3223 v2#429

Closed
HustonMmmavr wants to merge 7 commits into
adb-6.x-devfrom
ADDBDEB-3223-v2
Closed

Addbdeb 3223 v2#429
HustonMmmavr wants to merge 7 commits into
adb-6.x-devfrom
ADDBDEB-3223-v2

Conversation

@HustonMmmavr

Copy link
Copy Markdown

Here are some reminders before you submit the pull request

  • Add tests for the change
  • Document changes
  • Communicate in the mailing list if needed
  • Pass make installcheck
  • Review a PR in return to support the community

- no need to add addititonal check at exec scrpt
- no need to warn about setting segemnt value at create extension
  if smth goes wrong leader catches the exception at exec scrpt and
  run the create stmt with state end (cleanup)
- no need to have an if stmt at ApplyExtension ... because this logic is moved
  to exec scrpt for qe/qd utility. so all needed (previously code duplicate)
  setting of variables are set from part of exec scrpt
- todo check that big if stmt was correctly splitted
- todo check that Exec is correctly handled
  especially in case of create extension from unpackaged
- there is no need to have a break condition in a loop
  of ApplyUpdate for qe - at the beginnin of function
  assert
to fix problem - when railses segment
coordinator raises too
* QE may call this function with an empty updateVersions (like at postgres) - noop
*/
AssertImply(Gp_role == GP_ROLE_EXECUTE, list_length(updateVersions) == 1);
AssertImply(Gp_role == GP_ROLE_EXECUTE, list_length(updateVersions) <= 1);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

no, if segment will call this function without command from QD (from CreateExtension) here will be a problem - updateVersions list at QE is the same as at QD, so for loop works and it leads to errors

@HustonMmmavr

Copy link
Copy Markdown
Author

Duplicate of #430

@HustonMmmavr HustonMmmavr marked this as a duplicate of #430 Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant