Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
99168: randgen: disable generation of REGNAMESPACE type expressions r=msirek a=msirek

Casting an OID to REGNAMESPACE runs this SQL using the internal executor:
```
SELECT pg_namespace.oid, nspname FROM pg_catalog.pg_namespace WHERE oid = $1
```
When the `GenerateConstrainedScans` rule is disabled, this returns no
rows and so doesn't apply the cast. The same SQL, not run via an
internal executor, but also with `GenerateConstrainedScans` disabled,
behaves correctly. Disabling `GenerateConstrainedScans` all the time
prevents the cluster from starting.

The reason a full scan of pg_namespace doesn't find the OID is because it
only checks for schemas in the current database:
https://github.com/cockroachdb/cockroach/blob/7b341ffa678e4a22416e2274351fd56f415f5421/pkg/sql/virtual_schema.go#L602
https://github.com/cockroachdb/cockroach/blob/d10c3dd42c3dc40cad82792a30ae47fd2a663f43/pkg/sql/pg_catalog.go#L2099-L2106
https://github.com/cockroachdb/cockroach/blob/a7e9c4a68b81436d1f9382518d4267f74cbdac94/pkg/sql/information_schema.go#L2295-L2296

Whereas with a constrained scan, all descriptors are searched:
https://github.com/cockroachdb/cockroach/blob/af6a72a622ae05f3733b5db637403b3eaa9455f1/pkg/sql/catalog/descs/descriptor.go#L168-L175

The correct result is from the constrained scan, because we currently
allow cross-database references. Once cockroachdb#55791 is fixed, both results
should match.

The fix alternatives are:
1. disallow disabling of the `GenerateConstrainedScans` rule for internal SQL
2. turn off generation of REGNAMESPACE expressions in randgen to avoid hitting this problem in tests.

The 2nd fix alternative is implemented to avoid losing any of the
rule-disabling test coverage provided by the
`testing_optimizer_disable_rule_probability` setting.

Fixes cockroachdb#98322

100652: cluster-ui: fix cached data invalidation on timescale change r=xinhaoz a=xinhaoz

In a prior change, we moved the invalidation of cached data depending on the timescale to the local storage saga for CC. This was so invaldiation would occur after updating the cache. The local storage saga created for the time scale action was not hooked up to fire after the action, thus the data would not have been invalidated. This commit properly subscribes the saga to the update time scale action in CC.

Epic: none

Release note: None

100760: build: make sure toolchain has correct arguments for linking shared lib r=rail a=rickystewart

`geos` has been failing to build without this change since cockroachdb#100313. This fixes it.

Epic: none
Release note: None

Co-authored-by: Mark Sirek <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
4 people committed Apr 5, 2023
4 parents 42abc0c + 85b666e + fcdd4ce + e512e3d commit 530100f
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 8 deletions.
1 change: 1 addition & 0 deletions build/toolchains/crosstool-ng/cc_toolchain_config.bzl.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ all_compile_actions = [
]

all_link_actions = [
ACTION_NAMES.cpp_link_dynamic_library,
ACTION_NAMES.cpp_link_executable,
]

Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/randgen/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ var (
func init() {
for _, typ := range types.OidToType {
switch typ.Oid() {
case oid.T_regnamespace:
// Temporarily don't include this.
// TODO(msirek): Remove this exclusion once
// https://github.com/cockroachdb/cockroach/issues/55791 is fixed.
case oid.T_unknown, oid.T_anyelement:
// Don't include these.
case oid.T_anyarray, oid.T_oidvector, oid.T_int2vector:
Expand Down
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
StmtInsightEvent,
} from "src/insights";
import moment from "moment";
import { INTERNAL_APP_NAME_PREFIX } from "src/recentExecutions/recentStatementUtils";
import { INTERNAL_APP_NAME_PREFIX } from "src/util/constants";
import { FixFingerprintHexValue } from "../util";
import { getContentionDetailsApi } from "./contentionApi";

Expand Down
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
stmtInsightsByTxnExecutionQuery,
StmtInsightsResponseRow,
} from "./stmtInsightsApi";
import { INTERNAL_APP_NAME_PREFIX } from "src/recentExecutions/recentStatementUtils";
import { INTERNAL_APP_NAME_PREFIX } from "src/util/constants";
import { getContentionDetailsApi } from "./contentionApi";

export const TXN_QUERY_PREVIEW_MAX = 800;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import {
import { commonStyles } from "../common";
import { Loading } from "src";
import LoadingError from "../sqlActivity/errorComponent";
import { INTERNAL_APP_NAME_PREFIX } from "../recentExecutions/recentStatementUtils";
import { INTERNAL_APP_NAME_PREFIX } from "src/util/constants";
import { filteredStatementsData } from "../sqlActivity/util";

const cx = classNames.bind(styles);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ type Payload = {
value: any;
};

type TypedPayload<T> = {
export type TypedPayload<T> = {
value: T;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import { expectSaga, testSaga } from "redux-saga-test-plan";
import { actions } from "./localStorage.reducer";
import { actions as stmtInsightActions } from "src/store/insights/statementInsights/statementInsights.reducer";
import { actions as txnInsightActions } from "src/store/insights/transactionInsights/transactionInsights.reducer";
import { actions as sqlStatsActions } from "src/store/sqlStats/sqlStats.reducer";
import { actions as txnStatsActions } from "src/store/transactionStats";
import {
localStorageSaga,
updateLocalStorageItemSaga,
updateTimeScale,
} from "./localStorage.saga";
import { defaultTimeScaleSelected } from "../../timeScaleDropdown";
import { takeEvery, takeLatest } from "redux-saga/effects";

const ts = defaultTimeScaleSelected;

describe("local storage sagas", () => {
describe("localStorageSaga", () => {
it("should fork relevant sagas on actions", () => {
testSaga(localStorageSaga)
.next()
.all([
takeEvery(actions.update, updateLocalStorageItemSaga),
takeLatest(actions.updateTimeScale, updateTimeScale),
])
.finish()
.isDone();
});
});

describe("updateTimeScale", () => {
it("invalidates data depending on timescale ", () => {
return expectSaga(updateTimeScale, actions.updateTimeScale({ value: ts }))
.put(sqlStatsActions.invalidated())
.put(stmtInsightActions.invalidated())
.put(txnInsightActions.invalidated())
.put(txnStatsActions.invalidated())
.run();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,17 @@

import { AnyAction } from "redux";
import { all, call, takeEvery, takeLatest, put } from "redux-saga/effects";
import { actions } from "./localStorage.reducer";
import {
actions,
LocalStorageKeys,
TypedPayload,
} from "./localStorage.reducer";
import { actions as sqlStatsActions } from "src/store/sqlStats";
import { actions as stmtInsightActions } from "src/store/insights/statementInsights";
import { actions as txnInsightActions } from "src/store/insights/transactionInsights";
import { actions as txnStatsActions } from "src/store/transactionStats";
import { PayloadAction } from "@reduxjs/toolkit";
import { TimeScale } from "src/timeScaleDropdown";

export function* updateLocalStorageItemSaga(action: AnyAction) {
const { key, value } = action.payload;
Expand All @@ -25,18 +31,25 @@ export function* updateLocalStorageItemSaga(action: AnyAction) {
);
}

export function* updateTimeScale(action: AnyAction) {
export function* updateTimeScale(
action: PayloadAction<TypedPayload<TimeScale>>,
) {
yield all([
put(sqlStatsActions.invalidated()),
put(stmtInsightActions.invalidated()),
put(txnInsightActions.invalidated()),
put(txnStatsActions.invalidated()),
]);
yield call(
{ context: localStorage, fn: localStorage.setItem },
LocalStorageKeys.GLOBAL_TIME_SCALE,
JSON.stringify(action.payload?.value),
);
}

export function* localStorageSaga() {
yield all([
takeEvery(actions.update, updateLocalStorageItemSaga),
takeLatest(actions.updateTimeScale, updateLocalStorageItemSaga),
takeLatest(actions.updateTimeScale, updateTimeScale),
]);
}
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/cluster-ui/src/store/utils/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// licenses/APL.txt.

import { createSelector } from "reselect";
import { LocalStorageKeys } from "../localStorage";
import { LocalStorageKeys } from "src/store/localStorage/localStorage.reducer";
import { AppState } from "../reducers";

export const adminUISelector = createSelector(
Expand Down
2 changes: 2 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,5 @@ export const serverToClientErrorMessageMap = new Map([
]);

export const NO_SAMPLES_FOUND = "no samples";

export const INTERNAL_APP_NAME_PREFIX = "$ internal";

0 comments on commit 530100f

Please sign in to comment.