Skip to content

Commit

Permalink
Merge #68429 #68486
Browse files Browse the repository at this point in the history
68429: [CRDB-8149] Add CES survey link r=Santamaura a=Santamaura

The goal of this PR is to create a basic component that can open a link to the CES feedback survey. The current query parameters that are passed through are `clusterId` and `clusterVersion`. If there are any more query parameters that need to be included feel free to comment what they might be. The component will render empty (not able to open the survey) if for some reason we are unable to get the clusterId or clusterVersion values.

Below are some screenshots showcasing the feedback survey link and what the url looks like when opened in a local dev environment:

![image](https://user-images.githubusercontent.com/17861665/128223481-2e6b5e6d-77de-4e3e-8a8e-125171f42500.png)


![image](https://user-images.githubusercontent.com/17861665/128223593-c0350d3e-0104-4e1a-ab73-4f4baa939554.png)

This PR is in response to [this github issue](#66615)

68486: sql: fix accidental FK check skips r=RaduBerinde a=RaduBerinde

This change fixes a bug in the insert fast path where we accidentally
skip subsequent FK checks when a FK check can be skipped due to NULL
value.

Note that this bug does not manifest when optbuilder can determine
that the check can be elided entirely; notably, this is always the
case for non-prepared statements which insert a single row.

Fixes #68307.

Release note (bug fix): fixed missing foreign key checks in some cases
when there are multiple checks and the inserted data contains a NULL
for one of the checks.

Co-authored-by: Santamaura <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
  • Loading branch information
3 people committed Aug 5, 2021
3 parents c614690 + 5c5067f + 3eb6fb2 commit ca77dc5
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/sql/insert_fast_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ func (r *insertFastPathRun) addFKChecks(
return c.errorForRow(inputRow)
}
// We have a row with only NULLS, or a row with some NULLs and match
// method PARTIAL. We can ignore this row.
return nil
// method PARTIAL. We can skip this FK check for this row.
continue
}

span, err := c.generateSpan(inputRow)
Expand Down
17 changes: 17 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/fk
Original file line number Diff line number Diff line change
Expand Up @@ -3791,3 +3791,20 @@ statement error insert on table "reference_constraint_different_order_child" vio
INSERT
INTO reference_constraint_different_order_child
VALUES (99, 1, 2, 3, 'child_val')

# Regression test for #68307.
statement ok
CREATE TABLE t1 (a INT8 PRIMARY KEY);
CREATE TABLE t2 (a INT8 PRIMARY KEY);
CREATE TABLE t3 (a INT8 PRIMARY KEY, b INT8 REFERENCES t1 (a), c INT8 REFERENCES t2 (a));
INSERT INTO t1 VALUES (1);
INSERT INTO t2 VALUES (1);

statement error violates foreign key constraint "fk_b_ref_t1"
INSERT INTO t3 VALUES (1, 1, 1), (2, 2, NULL)

statement error violates foreign key constraint "fk_c_ref_t2"
INSERT INTO t3 VALUES (1, 1, 1), (2, NULL, 2)

statement ok
DROP TABLE t3, t1, t2
4 changes: 4 additions & 0 deletions pkg/ui/assets/external-link.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@
order 1

.right-side-panel
display flex
order 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2019 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.

@require '~src/components/core/index.styl'

.feedback-survey-link
display flex
flex-direction row
font-family $font-family--base
align-items center
border-right 1px solid #C0C6D9
margin-right $spacing-smaller
cursor pointer
&__title
margin-right $spacing-x-small
.image-container
width 32px
height 32px
display flex
justify-content center
align-items center
border-radius 16px
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2021 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 React from "react";
import { useSelector } from "react-redux";

import externalLinkIcon from "!!raw-loader!assets/external-link.svg";
import { trustIcon } from "src/util/trust";
import {
singleVersionSelector,
clusterIdSelector,
} from "../../../../redux/nodes";
import "./feedbackSurveyLink.styl";

const FeedBackSurveyLink = () => {
const singleVersion = useSelector(singleVersionSelector);
const clusterId = useSelector(clusterIdSelector);
const feedbackLink = new URL("https://www.cockroachlabs.com/survey/");
feedbackLink.searchParams.append("clusterId", clusterId);
feedbackLink.searchParams.append("clusterVersion", singleVersion);
if (!clusterId || !singleVersion) {
return <></>;
}
return (
<div
className="feedback-survey-link"
onClick={() => window.open(feedbackLink.toString())}
>
<div
className="image-container"
title="Share Feedback"
dangerouslySetInnerHTML={trustIcon(externalLinkIcon)}
/>
<div className="feedback-survey-link__title">Share feedback</div>
</div>
);
};

export default FeedBackSurveyLink;
2 changes: 2 additions & 0 deletions pkg/ui/src/views/app/containers/layout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from "src/redux/nodes";
import { AdminUIState } from "src/redux/state";
import LoginIndicator from "src/views/app/components/loginIndicator";
import FeedbackSurveyLink from "src/views/app/components/feedbackSurveyLink/feedbackSurveyLink";
import {
GlobalNavigation,
CockroachLabsLockupIcon,
Expand Down Expand Up @@ -80,6 +81,7 @@ class Layout extends React.Component<LayoutProps & RouteComponentProps> {
<CockroachLabsLockupIcon height={26} />
</Left>
<Right>
<FeedbackSurveyLink />
<LoginIndicator />
</Right>
</GlobalNavigation>
Expand Down

0 comments on commit ca77dc5

Please sign in to comment.