Skip to content

Commit

Permalink
Merge #41323 #41438
Browse files Browse the repository at this point in the history 
41323: sql: bevy of fixes for ActiveRecord compatibility r=jordanlewis a=jordanlewis

- improve efficiency of `'foo'::REGCLASS`
- improve efficiency of `pg_get_coldesc`
- parse INTERVAL(6) as a no-op meaning INTERVAL
- bugfix to generate_subscripts for oidvector and int2vector
- add pg_available_extensions
- fix `pg_get_indexdef` and `pg_attrdef.adbin` to be more Postgres compatible

With these fixes, 90% of the fork in `activerecord-cockroachdb-adapter` can go away.

41438: kv: fix cluster_logical_timestamp() r=andreimatei a=andreimatei

Before this patch, the txn.CommitTimestamp() function (which powers
the cluster_logical_timestamp(), among others) was failing to take into
consideration possible refreshes that might have happened since the
current epoch began (i.e. txn.RefreshedTimestamp).

Fixes #41424

Release note (bug fix): Fix a bug causing the
cluster_logical_timestamp() function to sometimes return incorrect
results.

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
  • Loading branch information
@jordanlewis @andreimatei
3 people committed Oct 8, 2019
3 parents d789ef3 + 72ff26e + b491dde commit c3593b0
Show file tree
Hide file tree
Showing 23 changed files with 343 additions and 158 deletions.
6 changes: 4 additions & 2 deletions pkg/ccl/importccl/import_table_creation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,10 @@ func (so *importSequenceOperators) ParseQualifiedTableName(
}

// Implements the tree.EvalDatabase interface.
func (so *importSequenceOperators) ResolveTableName(ctx context.Context, tn *tree.TableName) error {
return errSequenceOperators
func (so *importSequenceOperators) ResolveTableName(
ctx context.Context, tn *tree.TableName,
) (tree.ID, error) {
return 0, errSequenceOperators
}

// Implements the tree.EvalDatabase interface.
Expand Down
5 changes: 4 additions & 1 deletion pkg/kv/txn_coord_sender.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1187,8 +1187,11 @@ func (tc *TxnCoordSender) OrigTimestamp() hlc.Timestamp {
func (tc *TxnCoordSender) CommitTimestamp() hlc.Timestamp {
tc.mu.Lock()
defer tc.mu.Unlock()
txn := &tc.mu.txn
tc.mu.txn.OrigTimestampWasObserved = true
return tc.mu.txn.OrigTimestamp
commitTS := txn.OrigTimestamp
commitTS.Forward(txn.RefreshedTimestamp)
return commitTS
}

// CommitTimestampFixed is part of the client.TxnSender interface.
Expand Down
50 changes: 50 additions & 0 deletions pkg/kv/txn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)

// TestTxnDBBasics verifies that a simple transaction can be run and
Expand Down Expand Up @@ -589,3 +590,52 @@ func TestTxnResolveIntentsFromMultipleEpochs(t *testing.T) {
}
}
}

// Test that txn.CommitTimestamp() reflects refreshes.
func TestTxnCommitTimestampAdvancedByRefresh(t *testing.T) {
defer leaktest.AfterTest(t)()
ctx := context.Background()

// We're going to inject an uncertainty error, expect the refresh to succeed,
// and then check that the txn.CommitTimestamp() value reflects the refresh.
injected := false
var refreshTS hlc.Timestamp
errKey := roachpb.Key("inject_err")
s := createTestDBWithContextAndKnobs(t, client.DefaultDBContext(), &storage.StoreTestingKnobs{
TestingRequestFilter: func(ba roachpb.BatchRequest) *roachpb.Error {
if g, ok := ba.GetArg(roachpb.Get); ok && g.(*roachpb.GetRequest).Key.Equal(errKey) {
if injected {
return nil
}
injected = true
txn := ba.Txn.Clone()
refreshTS = txn.Timestamp.Add(0, 1)
pErr := roachpb.NewReadWithinUncertaintyIntervalError(
txn.OrigTimestamp,
refreshTS,
txn)
return roachpb.NewErrorWithTxn(pErr, txn)
}
return nil
},
})
defer s.Stop()

err := s.DB.Txn(ctx, func(ctx context.Context, txn *client.Txn) error {
_, err := txn.Get(ctx, errKey)
if err != nil {
return err
}
if !injected {
return errors.Errorf("didn't inject err")
}
commitTS := txn.CommitTimestamp()
// We expect to have refreshed just after the timestamp injected by the error.
expTS := refreshTS.Add(0, 1)
if !commitTS.Equal(expTS) {
return errors.Errorf("expected refreshTS: %s, got: %s", refreshTS, commitTS)
}
return nil
})
require.NoError(t, err)
}
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/builtin_function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1922,7 +1922,7 @@ CREATE TABLE test.pg_indexdef_test (a INT, UNIQUE INDEX pg_indexdef_idx (a ASC),
query T
SELECT pg_catalog.pg_get_indexdef((SELECT oid from pg_class WHERE relname='pg_indexdef_idx'))
----
CREATE UNIQUE INDEX pg_indexdef_idx ON test.public.pg_indexdef_test (a ASC)
CREATE UNIQUE INDEX pg_indexdef_idx ON test.public.pg_indexdef_test USING btree (a ASC)

query T
SELECT pg_catalog.pg_get_indexdef(0, 0, true)
Expand All @@ -1932,15 +1932,15 @@ NULL
query T
SELECT pg_catalog.pg_get_indexdef((SELECT oid from pg_class WHERE relname='pg_indexdef_idx'), 0, true)
----
CREATE UNIQUE INDEX pg_indexdef_idx ON test.public.pg_indexdef_test (a ASC)
CREATE UNIQUE INDEX pg_indexdef_idx ON test.public.pg_indexdef_test USING btree (a ASC)

statement ok
CREATE TABLE test.pg_indexdef_test_cols (a INT, b INT, UNIQUE INDEX pg_indexdef_cols_idx (a ASC, b DESC), INDEX other (a DESC))

query T
SELECT pg_catalog.pg_get_indexdef((SELECT oid from pg_class WHERE relname='pg_indexdef_cols_idx'), 0, true)
----
CREATE UNIQUE INDEX pg_indexdef_cols_idx ON test.public.pg_indexdef_test_cols (a ASC, b DESC)
CREATE UNIQUE INDEX pg_indexdef_cols_idx ON test.public.pg_indexdef_test_cols USING btree (a ASC, b DESC)

query T
SELECT pg_catalog.pg_get_indexdef((SELECT oid from pg_class WHERE relname='pg_indexdef_cols_idx'), 1, true)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/testdata/logic_test/grant_table
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ test pg_catalog pg_am public S
test pg_catalog pg_attrdef public SELECT
test pg_catalog pg_attribute public SELECT
test pg_catalog pg_auth_members public SELECT
test pg_catalog pg_available_extensions public SELECT
test pg_catalog pg_cast public SELECT
test pg_catalog pg_class public SELECT
test pg_catalog pg_collation public SELECT
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/information_schema
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ pg_catalog pg_am
pg_catalog pg_attrdef
pg_catalog pg_attribute
pg_catalog pg_auth_members
pg_catalog pg_available_extensions
pg_catalog pg_cast
pg_catalog pg_class
pg_catalog pg_collation
Expand Down Expand Up @@ -410,6 +411,7 @@ pg_am
pg_attrdef
pg_attribute
pg_auth_members
pg_available_extensions
pg_cast
pg_class
pg_collation
Expand Down Expand Up @@ -554,6 +556,7 @@ system pg_catalog pg_am SYSTEM VIE
system pg_catalog pg_attrdef SYSTEM VIEW NO 1
system pg_catalog pg_attribute SYSTEM VIEW NO 1
system pg_catalog pg_auth_members SYSTEM VIEW NO 1
system pg_catalog pg_available_extensions SYSTEM VIEW NO 1
system pg_catalog pg_cast SYSTEM VIEW NO 1
system pg_catalog pg_class SYSTEM VIEW NO 1
system pg_catalog pg_collation SYSTEM VIEW NO 1
Expand Down Expand Up @@ -1354,6 +1357,7 @@ NULL public system pg_catalog pg_am
NULL public system pg_catalog pg_attrdef SELECT NULL YES
NULL public system pg_catalog pg_attribute SELECT NULL YES
NULL public system pg_catalog pg_auth_members SELECT NULL YES
NULL public system pg_catalog pg_available_extensions SELECT NULL YES
NULL public system pg_catalog pg_cast SELECT NULL YES
NULL public system pg_catalog pg_class SELECT NULL YES
NULL public system pg_catalog pg_collation SELECT NULL YES
Expand Down Expand Up @@ -1644,6 +1648,7 @@ NULL public system pg_catalog pg_am
NULL public system pg_catalog pg_attrdef SELECT NULL YES
NULL public system pg_catalog pg_attribute SELECT NULL YES
NULL public system pg_catalog pg_auth_members SELECT NULL YES
NULL public system pg_catalog pg_available_extensions SELECT NULL YES
NULL public system pg_catalog pg_cast SELECT NULL YES
NULL public system pg_catalog pg_class SELECT NULL YES
NULL public system pg_catalog pg_collation SELECT NULL YES
Expand Down
29 changes: 27 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/orms
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ GROUP BY i.relname,
ORDER BY i.relname
----
name primary unique indkey column_indexes column_names definition
customers_id_idx false false 2 {1,2} {name,id} CREATE INDEX customers_id_idx ON test.public.customers (id ASC)
primary true true 1 {1,2} {name,id} CREATE UNIQUE INDEX "primary" ON test.public.customers (name ASC)
customers_id_idx false false 2 {1,2} {name,id} CREATE INDEX customers_id_idx ON test.public.customers USING btree (id ASC)
primary true true 1 {1,2} {name,id} CREATE UNIQUE INDEX "primary" ON test.public.customers USING btree (name ASC)


query TT colnames
Expand Down Expand Up @@ -137,3 +137,28 @@ AND t.relname = 'b'
AND i.relnamespace IN (SELECT oid FROM pg_namespace WHERE nspname = ANY (current_schemas(false)))
----
1

statement ok
CREATE TABLE c (a INT, b INT, PRIMARY KEY (a, b))

# ActiveRecord query for determining primary key cols.
query T
SELECT
a.attname
FROM
(
SELECT
indrelid, indkey, generate_subscripts(indkey, 1) AS idx
FROM
pg_index
WHERE
indrelid = '"c"'::REGCLASS AND indisprimary
)
AS i
JOIN pg_attribute AS a ON
a.attrelid = i.indrelid AND a.attnum = i.indkey[i.idx]
ORDER BY
i.idx
----
a
b
Loading

0 comments on commit c3593b0

Please sign in to comment.