Report unsupported database options under unsupported query constructs #2270
Conversation
priyanshi-yb
requested review from
sanyamsinghal and
makalaaneesh
and removed request for
sanyamsinghal
| } | ||
|
|
||
| var databaseOptionsPG17Issue = issue.Issue{ | ||
| Type: DATABASE_OPTIONS, |
There was a problem hiding this comment.
let's have a different type for both these issues? Type should ideally be unique. Perhaps, name can be the same.
| switch { | ||
| case testYbVersion.ReleaseType() == ybversion.V2_25_0_0.ReleaseType() && testYbVersion.GreaterThanOrEqual(ybversion.V2_25_0_0): | ||
| assert.NoError(t, err) | ||
| assertErrorCorrectlyThrownForIssueForYBVersion(t, fmt.Errorf(""), "", databaseOptionsPG15Issue) |
There was a problem hiding this comment.
assertNoError, is enough, right? do we need the assertErrorCorrectlyThrownForIssueForYBVersion as well?
There was a problem hiding this comment.
In this function assertErrorCorrectlyThrownForIssueForYBVersion we also do the IsFixedIn check so I think it makes sense to run this as well?
|
|
||
| defer conn.Close(context.Background()) | ||
| _, err = conn.Exec(ctx, sql) | ||
| switch { |
There was a problem hiding this comment.
Wait, why do we need this switch case? If it's 2.25, err will be empty, and assertErrorCorrectlyThrownForIssueForYBVersion will take care of it automatically, right?
There was a problem hiding this comment.
we need the switch case right, because firstly we are not marking this as fixed in 2.25 but the CREATE DATABASE works without error in the 2.25 so, we need to run this func assertErrorCorrectlyThrownForIssueForYBVersion with different expected and actual errors in different cases
There was a problem hiding this comment.
let's add a comment here as well to indicate that it is not actually supported in 2.25, but does not throw any error as such
| Type: DATABASE_OPTIONS_PG15, | ||
| Name: "Database options", | ||
| Impact: constants.IMPACT_LEVEL_2, | ||
| Description: "Database options (%s) introduced in PostgreSQL 12 and later are not supported yet in YugabyteDB.", |
There was a problem hiding this comment.
lets define this along with other description constant vars?
| func ProtoAsCreateDBStmtNode(msg protoreflect.Message) (*pg_query.CreatedbStmt, error) { | ||
| node, ok := msg.Interface().(*pg_query.CreatedbStmt) | ||
| if !ok { | ||
| return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_CREATEDB_STMT_NODE) | ||
| } | ||
| return node, nil | ||
| } |
There was a problem hiding this comment.
hm yeah, missed removing it
priyanshi-yb
force-pushed
the
priyanshi/database-options
branch
from
e10375e to
9d58b8c
Compare
|
|
||
| defer conn.Close(context.Background()) | ||
| _, err = conn.Exec(ctx, sql) | ||
| switch { |
There was a problem hiding this comment.
let's add a comment here as well to indicate that it is not actually supported in 2.25, but does not throw any error as such
Describe the changes in this pull request
Reporting the
CREATE DATABASE WITH [OPTIONS...]for the options not unsupported in >=PG12.fixes #1994
Added advisory-locks, system-columns tests in issues-integrations tests
Removed 2.18 from the YB version list in
issues-test.ymlworkflowDescribe if there are any user-facing changes
How was this pull request tested?
added unit tests and end-to-end test
Does your PR have changes that can cause upgrade issues?