Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pgwire: rework DecodeOidDatum to DecodeDatum to parse OidFamily types #56298

Merged
merged 2 commits into from
Nov 23, 2020

Conversation

otan
Copy link
Contributor

@otan otan commented Nov 4, 2020

Resolves #56193

pgwire: rework DecodeOidDatum to DecodeDatum to parse OidFamily types

Reworked DecodeOidDatum to DecodeDatum to take in a type, which
encodes additional useful information necessary for ENUMs and oid
family types.

Release note (bug fix): Fixed a bug where reg* types were not parsed
properly over pgwire, COPY or prepared statements.

tree: create ParseDOid method

  • Move ParseDOid and associated methods to new function.
  • Move datum_test.go to datum_integration_test.go, as it does not import
    datum.go tests.
  • Move some of datum_invariants_test.go out into datum_test.go.
  • Created new datum_test.go with pure unit tests.

Release note: None

@otan otan requested review from rafiss and a team November 4, 2020 21:23
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@otan otan force-pushed the parse_oid branch 4 times, most recently from 4c367f4 to d9b38b9 Compare November 5, 2020 06:39
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

lgtm after fixing the comment. nice refactor ^_^

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @otan and @rafiss)


pkg/sql/pgwire/pgwirebase/encoding.go, line 300 at r2 (raw file):

If res is nil

nit: need to update comment since res is gone

@@ -203,77 +203,3 @@ func TestTupleCastVolatility(t *testing.T) {
}
}
}

func TestCastStringToRegClassTableName(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh just noticed, where did this test go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pkg/sql/sem/tree/datum_test.go

* Move ParseDOid and associated methods to new function.
* Move datum_test.go to datum_integration_test.go, as it does not import
  datum.go tests.
* Move some of datum_invariants_test.go out into datum_test.go.
* Created new datum_test.go with pure unit tests.

Release note: None
Reworked DecodeOidDatum to DecodeDatum to take in a type, which
encodes additional useful information necessary for ENUMs and oid
family types.

Release note (bug fix): Fixed a bug where reg* types were not parsed
properly over pgwire, COPY or prepared statements.
@otan
Copy link
Contributor Author

otan commented Nov 23, 2020

bors r=rafiss

@craig
Copy link
Contributor

craig bot commented Nov 23, 2020

Build succeeded:

@craig craig bot merged commit c2b4fff into cockroachdb:master Nov 23, 2020
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.

Can't escape single quote ' in Primary key query
3 participants