Web console: Fix maxRowsPerSegment validation in hashed compaction spec (#11308)

* allow defining of maxRowsPerSegment for now

* use common util

* update snapshots

* fix test

* fix e2e test

web-console/e2e-tests/component/load-data/config/partition.ts

@@ -57,7 +57,11 @@ export class HashedPartitionsSpec implements PartitionsSpec {
   readonly type: string;
 
   static async read(page: playwright.Page): Promise<HashedPartitionsSpec> {
-    const numShards = await getLabeledInputAsNumber(page, HashedPartitionsSpec.NUM_SHARDS);
+    // The shards control may not be visible in that case this is not an error, it is simply not set (null)
+    let numShards: number | null = null;
+    try {
+      numShards = await getLabeledInputAsNumber(page, HashedPartitionsSpec.NUM_SHARDS);
+    } catch {}
     return new HashedPartitionsSpec({ numShards });
   }
 

web-console/src/components/auto-form/auto-form.spec.tsx

@@ -50,7 +50,8 @@ describe('AutoForm', () => {
           },
 
           { name: 'testNotDefined', type: 'string', defined: false },
-          { name: 'testAdvanced', type: 'string', hideInMore: true },
+          { name: 'testHide', type: 'string', hide: true },
+          { name: 'testHideInMore', type: 'string', hideInMore: true },
         ]}
         model={String}
         onChange={() => {}}

web-console/src/components/auto-form/auto-form.tsx

@@ -56,6 +56,7 @@ export interface Field<M> {
   disabled?: Functor<M, boolean>;
   defined?: Functor<M, boolean>;
   required?: Functor<M, boolean>;
+  hide?: Functor<M, boolean>;
   hideInMore?: Functor<M, boolean>;
   valueAdjustment?: (value: any) => any;
   adjustment?: (model: M) => M;
@@ -456,10 +457,15 @@ export class AutoForm<T extends Record<string, any>> extends React.PureComponent
     let shouldShowMore = false;
     const shownFields = fields.filter(field => {
       if (AutoForm.evaluateFunctor(field.defined, model, true)) {
+        if (AutoForm.evaluateFunctor(field.hide, model, false)) {
+          return false;
+        }
+
         if (AutoForm.evaluateFunctor(field.hideInMore, model, false)) {
           shouldShowMore = true;
           return showMore;
         }
+
         return true;
       } else {
         return false;

web-console/src/components/json-input/json-input.tsx

@@ -67,6 +67,7 @@ interface InternalValue {
 interface JsonInputProps {
   value: any;
   onChange: (value: any) => void;
+  onError?: (error: Error) => void;
   placeholder?: string;
   focus?: boolean;
   width?: string;
@@ -75,7 +76,7 @@ interface JsonInputProps {
 }
 
 export const JsonInput = React.memo(function JsonInput(props: JsonInputProps) {
-  const { onChange, placeholder, focus, width, height, value, issueWithValue } = props;
+  const { onChange, onError, placeholder, focus, width, height, value, issueWithValue } = props;
   const [internalValue, setInternalValue] = useState<InternalValue>(() => ({
     value,
     stringified: stringifyJson(value),
@@ -120,7 +121,9 @@ export const JsonInput = React.memo(function JsonInput(props: JsonInputProps) {
             stringified: inputJson,
           });
 
-          if (!error) {
+          if (error) {
+            onError?.(error);
+          } else {
             onChange(value);
           }
 

web-console/src/dialogs/compaction-dialog/__snapshots__/compaction-dialog.spec.tsx.snap

@@ -86,6 +86,28 @@ exports[`CompactionDialog matches snapshot with compactionConfig (dynamic partit
               </p>
             </React.Fragment>,
             "name": "tuningConfig.partitionsSpec.targetRowsPerSegment",
+            "placeholder": "(defaults to 500000)",
+            "type": "number",
+            "zeroMeansUndefined": true,
+          },
+          Object {
+            "defined": [Function],
+            "info": <React.Fragment>
+              <p>
+                Target number of rows to include in a partition, should be a number that targets segments of 500MB~1GB.
+              </p>
+              <p>
+                <Unknown>
+                  maxRowsPerSegment
+                </Unknown>
+                 is an alias for 
+                <Unknown>
+                  targetRowsPerSegment
+                </Unknown>
+                . Only one of these properties can be used.
+              </p>
+            </React.Fragment>,
+            "name": "tuningConfig.partitionsSpec.maxRowsPerSegment",
             "type": "number",
             "zeroMeansUndefined": true,
           },
@@ -335,6 +357,28 @@ exports[`CompactionDialog matches snapshot with compactionConfig (hashed partiti
               </p>
             </React.Fragment>,
             "name": "tuningConfig.partitionsSpec.targetRowsPerSegment",
+            "placeholder": "(defaults to 500000)",
+            "type": "number",
+            "zeroMeansUndefined": true,
+          },
+          Object {
+            "defined": [Function],
+            "info": <React.Fragment>
+              <p>
+                Target number of rows to include in a partition, should be a number that targets segments of 500MB~1GB.
+              </p>
+              <p>
+                <Unknown>
+                  maxRowsPerSegment
+                </Unknown>
+                 is an alias for 
+                <Unknown>
+                  targetRowsPerSegment
+                </Unknown>
+                . Only one of these properties can be used.
+              </p>
+            </React.Fragment>,
+            "name": "tuningConfig.partitionsSpec.maxRowsPerSegment",
             "type": "number",
             "zeroMeansUndefined": true,
           },
@@ -584,6 +628,28 @@ exports[`CompactionDialog matches snapshot with compactionConfig (single_dim par
               </p>
             </React.Fragment>,
             "name": "tuningConfig.partitionsSpec.targetRowsPerSegment",
+            "placeholder": "(defaults to 500000)",
+            "type": "number",
+            "zeroMeansUndefined": true,
+          },
+          Object {
+            "defined": [Function],
+            "info": <React.Fragment>
+              <p>
+                Target number of rows to include in a partition, should be a number that targets segments of 500MB~1GB.
+              </p>
+              <p>
+                <Unknown>
+                  maxRowsPerSegment
+                </Unknown>
+                 is an alias for 
+                <Unknown>
+                  targetRowsPerSegment
+                </Unknown>
+                . Only one of these properties can be used.
+              </p>
+            </React.Fragment>,
+            "name": "tuningConfig.partitionsSpec.maxRowsPerSegment",
             "type": "number",
             "zeroMeansUndefined": true,
           },
@@ -833,6 +899,28 @@ exports[`CompactionDialog matches snapshot without compactionConfig 1`] = `
               </p>
             </React.Fragment>,
             "name": "tuningConfig.partitionsSpec.targetRowsPerSegment",
+            "placeholder": "(defaults to 500000)",
+            "type": "number",
+            "zeroMeansUndefined": true,
+          },
+          Object {
+            "defined": [Function],
+            "info": <React.Fragment>
+              <p>
+                Target number of rows to include in a partition, should be a number that targets segments of 500MB~1GB.
+              </p>
+              <p>
+                <Unknown>
+                  maxRowsPerSegment
+                </Unknown>
+                 is an alias for 
+                <Unknown>
+                  targetRowsPerSegment
+                </Unknown>
+                . Only one of these properties can be used.
+              </p>
+            </React.Fragment>,
+            "name": "tuningConfig.partitionsSpec.maxRowsPerSegment",
             "type": "number",
             "zeroMeansUndefined": true,
           },

web-console/src/dialogs/compaction-dialog/compaction-dialog.tsx

@@ -42,12 +42,10 @@ export const CompactionDialog = React.memo(function CompactionDialog(props: Comp
       tuningConfig: { partitionsSpec: { type: 'dynamic' } },
     },
   );
+  const [jsonError, setJsonError] = useState<Error | undefined>();
 
   const issueWithCurrentConfig = AutoForm.issueWithModel(currentConfig, COMPACTION_CONFIG_FIELDS);
-  function handleSubmit() {
+  const disableSubmit = Boolean(jsonError || issueWithCurrentConfig);
-    if (issueWithCurrentConfig) return;
-    onSave(currentConfig);
-  }
 
   return (
     <Dialog
@@ -68,7 +66,11 @@ export const CompactionDialog = React.memo(function CompactionDialog(props: Comp
         ) : (
           <JsonInput
             value={currentConfig}
-            onChange={setCurrentConfig}
+            onChange={v => {
+              setCurrentConfig(v);
+              setJsonError(undefined);
+            }}
+            onError={setJsonError}
             issueWithValue={value => AutoForm.issueWithModel(value, COMPACTION_CONFIG_FIELDS)}
             height="100%"
           />
@@ -81,8 +83,8 @@ export const CompactionDialog = React.memo(function CompactionDialog(props: Comp
           <Button
             text="Submit"
             intent={Intent.PRIMARY}
-            onClick={handleSubmit}
+            disabled={disableSubmit}
-            disabled={Boolean(issueWithCurrentConfig)}
+            onClick={() => onSave(currentConfig)}
           />
         </div>
       </div>

web-console/src/dialogs/coordinator-dynamic-config-dialog/__snapshots__/coordinator-dynamic-config-dialog.spec.tsx.snap

@@ -5,6 +5,7 @@ exports[`coordinator dynamic config matches snapshot 1`] = `
   className="coordinator-dynamic-config-dialog"
   onClose={[Function]}
   onSave={[Function]}
+  saveDisabled={false}
   title="Coordinator dynamic config"
 >
   <p>

web-console/src/dialogs/coordinator-dynamic-config-dialog/coordinator-dynamic-config-dialog.tsx

@@ -46,6 +46,7 @@ export const CoordinatorDynamicConfigDialog = React.memo(function CoordinatorDyn
   const { onClose } = props;
   const [currentTab, setCurrentTab] = useState<FormJsonTabs>('form');
   const [dynamicConfig, setDynamicConfig] = useState<CoordinatorDynamicConfig>({});
+  const [jsonError, setJsonError] = useState<Error | undefined>();
 
   const [historyRecordsState] = useQueryManager<null, any[]>({
     processQuery: async () => {
@@ -100,6 +101,7 @@ export const CoordinatorDynamicConfigDialog = React.memo(function CoordinatorDyn
   return (
     <SnitchDialog
       className="coordinator-dynamic-config-dialog"
+      saveDisabled={Boolean(jsonError)}
       onSave={saveConfig}
       onClose={onClose}
       title="Coordinator dynamic config"
@@ -121,7 +123,15 @@ export const CoordinatorDynamicConfigDialog = React.memo(function CoordinatorDyn
           onChange={setDynamicConfig}
         />
       ) : (
-        <JsonInput value={dynamicConfig} onChange={setDynamicConfig} height="50vh" />
+        <JsonInput
+          value={dynamicConfig}
+          height="50vh"
+          onChange={v => {
+            setDynamicConfig(v);
+            setJsonError(undefined);
+          }}
+          onError={setJsonError}
+        />
       )}
     </SnitchDialog>
   );

web-console/src/dialogs/lookup-edit-dialog/lookup-edit-dialog.tsx

@@ -58,6 +58,11 @@ export const LookupEditDialog = React.memo(function LookupEditDialog(props: Look
   } = props;
   const [currentTab, setCurrentTab] = useState<FormJsonTabs>('form');
   const [updateVersionOnSubmit, setUpdateVersionOnSubmit] = useState(true);
+  const [jsonError, setJsonError] = useState<Error | undefined>();
+
+  const disableSubmit = Boolean(
+    jsonError || isLookupInvalid(lookupName, lookupVersion, lookupTier, lookupSpec),
+  );
 
   return (
     <Dialog
@@ -122,10 +127,12 @@ export const LookupEditDialog = React.memo(function LookupEditDialog(props: Look
         ) : (
           <JsonInput
             value={lookupSpec}
+            height="80vh"
             onChange={m => {
               onChange('spec', m);
+              setJsonError(undefined);
             }}
-            height="80vh"
+            onError={setJsonError}
           />
         )}
       </div>
@@ -135,10 +142,10 @@ export const LookupEditDialog = React.memo(function LookupEditDialog(props: Look
           <Button
             text="Submit"
             intent={Intent.PRIMARY}
+            disabled={disableSubmit}
             onClick={() => {
               onSubmit(updateVersionOnSubmit && isEdit);
             }}
-            disabled={isLookupInvalid(lookupName, lookupVersion, lookupTier, lookupSpec)}
           />
         </div>
       </div>

web-console/src/druid-models/compaction-config.tsx

@@ -70,9 +70,11 @@ export const COMPACTION_CONFIG_FIELDS: Field<CompactionConfig>[] = [
     name: 'tuningConfig.partitionsSpec.targetRowsPerSegment',
     type: 'number',
     zeroMeansUndefined: true,
+    placeholder: `(defaults to 500000)`,
     defined: (t: CompactionConfig) =>
       deepGet(t, 'tuningConfig.partitionsSpec.type') === 'hashed' &&
-      !deepGet(t, 'tuningConfig.partitionsSpec.numShards'),
+      !deepGet(t, 'tuningConfig.partitionsSpec.numShards') &&
+      !deepGet(t, 'tuningConfig.partitionsSpec.maxRowsPerSegment'),
     info: (
       <>
         <p>
@@ -86,12 +88,34 @@ export const COMPACTION_CONFIG_FIELDS: Field<CompactionConfig>[] = [
       </>
     ),
   },
+  {
+    name: 'tuningConfig.partitionsSpec.maxRowsPerSegment',
+    type: 'number',
+    zeroMeansUndefined: true,
+    defined: (t: CompactionConfig) =>
+      deepGet(t, 'tuningConfig.partitionsSpec.type') === 'hashed' &&
+      !deepGet(t, 'tuningConfig.partitionsSpec.numShards') &&
+      !deepGet(t, 'tuningConfig.partitionsSpec.targetRowsPerSegment'),
+    info: (
+      <>
+        <p>
+          Target number of rows to include in a partition, should be a number that targets segments
+          of 500MB~1GB.
+        </p>
+        <p>
+          <Code>maxRowsPerSegment</Code> is an alias for <Code>targetRowsPerSegment</Code>. Only one
+          of these properties can be used.
+        </p>
+      </>
+    ),
+  },
   {
     name: 'tuningConfig.partitionsSpec.numShards',
     type: 'number',
     zeroMeansUndefined: true,
     defined: (t: CompactionConfig) =>
       deepGet(t, 'tuningConfig.partitionsSpec.type') === 'hashed' &&
+      !deepGet(t, 'tuningConfig.partitionsSpec.maxRowsPerSegment') &&
       !deepGet(t, 'tuningConfig.partitionsSpec.targetRowsPerSegment'),
     info: (
       <>

web-console/src/utils/general.tsx

@@ -22,6 +22,7 @@ import copy from 'copy-to-clipboard';
 import { SqlExpression, SqlFunction, SqlLiteral, SqlRef } from 'druid-query-toolkit';
 import FileSaver from 'file-saver';
 import hasOwnProp from 'has-own-prop';
+import * as JSONBig from 'json-bigint-native';
 import numeral from 'numeral';
 import React from 'react';
 import { Filter, FilterRender } from 'react-table';
@@ -382,3 +383,15 @@ export function moveElement<T>(items: readonly T[], fromIndex: number, toIndex:
     return items.slice();
   }
 }
+
+export function stringifyValue(value: unknown): string {
+  switch (typeof value) {
+    case 'object':
+      if (!value) return String(value);
+      if (typeof (value as any).toISOString === 'function') return (value as any).toISOString();
+      return JSONBig.stringify(value);
+
+    default:
+      return String(value);
+  }
+}

web-console/src/views/query-view/query-output/query-output.tsx

@@ -27,13 +27,12 @@ import {
   SqlRef,
   trimString,
 } from 'druid-query-toolkit';
-import * as JSONBig from 'json-bigint-native';
 import React, { useEffect, useState } from 'react';
 import ReactTable from 'react-table';
 
 import { BracedText, TableCell } from '../../../components';
 import { ShowValueDialog } from '../../../dialogs/show-value-dialog/show-value-dialog';
-import { copyAndAlert, deepSet, filterMap, prettyPrintSql } from '../../../utils';
+import { copyAndAlert, deepSet, filterMap, prettyPrintSql, stringifyValue } from '../../../utils';
 import { BasicAction, basicActionsToMenu } from '../../../utils/basic-action';
 
 import { ColumnRenameInput } from './column-rename-input/column-rename-input';
@@ -44,18 +43,6 @@ function isComparable(x: unknown): boolean {
   return x !== null && x !== '' && !isNaN(Number(x));
 }
 
-function stringifyValue(value: unknown): string {
-  switch (typeof value) {
-    case 'object':
-      if (!value) return String(value);
-      if (typeof (value as any).toISOString === 'function') return (value as any).toISOString();
-      return JSONBig.stringify(value);
-
-    default:
-      return String(value);
-  }
-}
-
 interface Pagination {
   page: number;
   pageSize: number;

web-console/src/views/query-view/query-view.spec.tsx

@@ -32,11 +32,20 @@ describe('QueryView', () => {
     expect(sqlView).toMatchSnapshot();
   });
 
-  it('trimSemicolon', () => {
+  it('.trimSemicolon', () => {
     expect(QueryView.trimSemicolon('SELECT * FROM tbl;')).toEqual('SELECT * FROM tbl');
     expect(QueryView.trimSemicolon('SELECT * FROM tbl;   ')).toEqual('SELECT * FROM tbl   ');
     expect(QueryView.trimSemicolon('SELECT * FROM tbl; --hello  ')).toEqual(
       'SELECT * FROM tbl --hello  ',
     );
   });
+
+  it('.formatStr', () => {
+    expect(QueryView.formatStr(null, 'csv')).toEqual('"null"');
+    expect(QueryView.formatStr('hello\nworld', 'csv')).toEqual('"hello world"');
+    expect(QueryView.formatStr(123, 'csv')).toEqual('"123"');
+    expect(QueryView.formatStr(new Date('2021-01-02T03:04:05.678Z'), 'csv')).toEqual(
+      '"2021-01-02T03:04:05.678Z"',
+    );
+  });
 });

web-console/src/views/query-view/query-view.tsx

@@ -48,6 +48,7 @@ import {
   QueryState,
   RowColumn,
   SemiJoinQueryExplanation,
+  stringifyValue,
 } from '../../utils';
 import { isEmptyContext, QueryContext } from '../../utils/query-context';
 import { QueryRecord, QueryRecordUtil } from '../../utils/query-history';
@@ -142,19 +143,16 @@ export class QueryView extends React.PureComponent<QueryViewProps, QueryViewStat
     }
   }
 
-  static formatStr(s: string | number, format: 'csv' | 'tsv') {
+  static formatStr(s: null | string | number | Date, format: 'csv' | 'tsv') {
+    // stringify and remove line break
+    const str = stringifyValue(s).replace(/(?:\r\n|\r|\n)/g, ' ');
+
     if (format === 'csv') {
-      // remove line break, single quote => double quote, handle ','
+      // csv: single quote => double quote, handle ','
-      return `"${String(s)
+      return `"${str.replace(/"/g, '""')}"`;
-        .replace(/(?:\r\n|\r|\n)/g, ' ')
-        .replace(/"/g, '""')}"`;
     } else {
-      // tsv
+      // tsv: single quote => double quote, \t => ''
-      // remove line break, single quote => double quote, \t => ''
+      return str.replace(/\t/g, '').replace(/"/g, '""');
-      return String(s)
-        .replace(/(?:\r\n|\r|\n)/g, ' ')
-        .replace(/\t/g, '')
-        .replace(/"/g, '""');
     }
   }