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.

Sign up for GitHub

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

Jump to bottom

ui: Disable tip for sampled queries #1471

Merged
merged 8 commits into from
Jul 8, 2024
Merged

ui: Disable tip for sampled queries #1471

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
859b709
ui: disable tip for sampled queries
zhangvi7 Jun 28, 2024
dfb909f
fix nodetest error
zhangvi7 Jun 28, 2024
f7f035c
address comments
zhangvi7 Jun 28, 2024
137efec
fix sampling bugs on query composer
zhangvi7 Jul 2, 2024
208f6d0
remove comment
zhangvi7 Jul 2, 2024
4558e08
address comment
zhangvi7 Jul 8, 2024
9583ef6
remove sampleRate const
zhangvi7 Jul 8, 2024
29c5d89
Merge branch 'pinterest:master' into master
zhangvi7 Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,7 @@ class DataDocQueryCellComponent extends React.PureComponent<IProps, IState> {
changeCellContext={isEditable ? this.handleChange : null}
onSamplingInfoClick={this.toggleShowTableSamplingInfoModal}
hasSamplingTables={this.hasSamplingTables}
sampleRate={this.sampleRate}
/>
);
}
Expand Down
3 changes: 3 additions & 0 deletions querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface IProps {

onSamplingInfoClick?: () => void;
hasSamplingTables?: boolean;
sampleRate: number;
}

export const DataDocQueryExecutions: React.FunctionComponent<IProps> =
Expand All @@ -38,6 +39,7 @@ export const DataDocQueryExecutions: React.FunctionComponent<IProps> =
isQueryCollapsed,
onSamplingInfoClick,
hasSamplingTables,
sampleRate,
}) => {
const { cellIdToExecutionId, onQueryCellSelectExecution } =
useContext(DataDocContext);
Expand Down Expand Up @@ -155,6 +157,7 @@ export const DataDocQueryExecutions: React.FunctionComponent<IProps> =
changeCellContext={changeCellContext}
onSamplingInfoClick={onSamplingInfoClick}
hasSamplingTables={hasSamplingTables}
sampleRate={sampleRate}
/>
);

Expand Down
48 changes: 33 additions & 15 deletions querybook/webapp/components/QueryComposer/QueryComposer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,14 +395,6 @@ function useTranspileQuery(
};
}

function getQueryExecutionMetadata(sampleRate: number) {
const metadata = {};
if (sampleRate > 0) {
metadata['sample_rate'] = sampleRate;
}
return Object.keys(metadata).length === 0 ? null : metadata;
}

const QueryComposer: React.FC = () => {
useTrackView(ComponentType.ADHOC_QUERY);
useBrowserTitle('Adhoc Query');
Expand Down Expand Up @@ -507,9 +499,34 @@ const QueryComposer: React.FC = () => {

const triggerSurvey = useSurveyTrigger();

const queryExecutionMetadata = getQueryExecutionMetadata(sampleRate);
const getSampleRate = useCallback(
() => (Object.keys(samplingTables).length > 0 ? sampleRate : -1),
[sampleRate, samplingTables]
);

const getSamplingTables = useCallback(() => {
Object.keys(samplingTables).forEach((tableName) => {
samplingTables[tableName].sample_rate = getSampleRate();
});
return samplingTables;
}, [getSampleRate, samplingTables]);
zhangvi7 marked this conversation as resolved.
Show resolved Hide resolved

const getQueryExecutionMetadata = useCallback(() => {
const metadata = {};

const sampleRate = getSampleRate();
if (sampleRate > 0) {
metadata['sample_rate'] = sampleRate;
}

return Object.keys(metadata).length === 0 ? null : metadata;
}, [getSampleRate]);

const handleRunQuery = React.useCallback(async () => {
const sampleRate = getSampleRate();
const samplingTables = getSamplingTables();
const queryExecutionMetadata = getQueryExecutionMetadata();

trackClick({
component: ComponentType.ADHOC_QUERY,
element: ElementType.RUN_QUERY_BUTTON,
Expand All @@ -520,6 +537,7 @@ const QueryComposer: React.FC = () => {
});
// Throttle to prevent double run
await sleep(250);

const transformedQuery = await transformQuery(
getCurrentSelectedQuery(),
engine.language,
Expand Down Expand Up @@ -553,16 +571,16 @@ const QueryComposer: React.FC = () => {
setResultsCollapsed(false);
}
}, [
getSampleRate,
getSamplingTables,
getQueryExecutionMetadata,
hasLintErrors,
sampleRate,
getCurrentSelectedQuery,
engine,
templatedVariables,
rowLimit,
samplingTables,
triggerSurvey,
dispatch,
queryExecutionMetadata,
setExecutionId,
]);

Expand Down Expand Up @@ -596,7 +614,6 @@ const QueryComposer: React.FC = () => {
if (table?.custom_properties?.sampling) {
samplingTables[tableName] = {
sampled_table: table.custom_properties?.sampled_table,
sample_rate: sampleRate,
};
}
});
Expand Down Expand Up @@ -666,6 +683,7 @@ const QueryComposer: React.FC = () => {
hasSamplingTables={
Object.keys(samplingTables).length > 0
}
sampleRate={sampleRate}
/>
</div>
</Resizable>
Expand All @@ -692,7 +710,7 @@ const QueryComposer: React.FC = () => {
<DataDocTableSamplingInfo
query={getCurrentSelectedQuery()}
language={engine.language}
samplingTables={samplingTables}
samplingTables={getSamplingTables()}
onHide={() => setShowTableSamplingInfoModal(false)}
/>
);
Expand Down Expand Up @@ -732,7 +750,7 @@ const QueryComposer: React.FC = () => {
rowLimit={rowLimit}
onRowLimitChange={setRowLimit}
hasSamplingTables={Object.keys(samplingTables).length > 0}
sampleRate={sampleRate}
sampleRate={getSampleRate()}
onSampleRateChange={setSampleRate}
onTableSamplingInfoClick={() =>
setShowTableSamplingInfoModal(true)
Expand Down
3 changes: 3 additions & 0 deletions querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ interface IProps {
id: number;
onSamplingInfoClick?: () => void;
hasSamplingTables?: boolean;
sampleRate: number;
}

export const QueryComposerExecution: React.FunctionComponent<IProps> = ({
id,
onSamplingInfoClick,
hasSamplingTables,
sampleRate,
}) => {
const execution = useSelector(
(state: IStoreState) => state.queryExecutions.queryExecutionById[id]
Expand Down Expand Up @@ -61,6 +63,7 @@ export const QueryComposerExecution: React.FunctionComponent<IProps> = ({
id={id}
onSamplingInfoClick={onSamplingInfoClick}
hasSamplingTables={hasSamplingTables}
sampleRate={sampleRate}
/>
</div>
);
Expand Down
3 changes: 3 additions & 0 deletions querybook/webapp/components/QueryExecution/QueryExecution.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ interface IProps {

onSamplingInfoClick?: () => void;
hasSamplingTables?: boolean;
sampleRate?: number;
}

function useQueryExecutionReduxState(queryId: number) {
Expand Down Expand Up @@ -92,6 +93,7 @@ export const QueryExecution: React.FC<IProps> = ({

onSamplingInfoClick,
hasSamplingTables,
sampleRate,
}) => {
const isEditable = useSelector((state: IStoreState) =>
canCurrentUserEditSelector(state, docId)
Expand Down Expand Up @@ -144,6 +146,7 @@ export const QueryExecution: React.FC<IProps> = ({
queryExecution={queryExecution}
onSamplingInfoClick={onSamplingInfoClick}
hasSamplingTables={hasSamplingTables}
sampleRate={sampleRate}
/>
);

Expand Down
13 changes: 9 additions & 4 deletions querybook/webapp/components/QueryExecution/SamplingToolTip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,28 @@ interface SamplingTooltipProps {
queryExecution: IQueryExecution;
onSamplingInfoClick?: () => void;
hasSamplingTables?: boolean;
sampleRate: number;
}

export const SamplingTooltip: React.FC<SamplingTooltipProps> = ({
queryExecution: { status, id },
onSamplingInfoClick,
hasSamplingTables,
sampleRate,
}) => {
const { enabled, sampling_tool_tip_delay: delay } =
PublicConfig.table_sampling;

const queryCanBeSampled = enabled && hasSamplingTables;

const [showSamplingTip, setShowSamplingTip] = useState(false);

// If query run longer than 10 seconds, we show a suggestion to users they can sample their query
useEffect(() => {
if (queryCanBeSampled && status <= QueryExecutionStatus.RUNNING) {
if (
enabled &&
hasSamplingTables &&
sampleRate <= 0 &&
status <= QueryExecutionStatus.RUNNING
) {
const timer = setTimeout(() => {
setShowSamplingTip(true);
}, delay);
Expand All @@ -38,7 +43,7 @@ export const SamplingTooltip: React.FC<SamplingTooltipProps> = ({
setShowSamplingTip(false);
};
}
}, [status, id, queryCanBeSampled, delay]);
}, [id, delay, enabled, hasSamplingTables, sampleRate, status]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is the current query_execution_id, I added it here so the effect reexecute when we run new query


const samplingTipDOM = showSamplingTip && (
<Message size="small" type="warning" className="SamplingToolTip">
Expand Down
Loading