Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 25 additions & 6 deletions amoro-web/src/views/tables/components/Details.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,21 @@ limitations under the License.
<script setup lang="ts">
import { computed, onMounted, reactive, shallowReactive, watch } from 'vue'
import { useI18n } from 'vue-i18n'
import { useRoute } from 'vue-router'
import { useRoute, useRouter } from 'vue-router'
import type { ColumnProps } from 'ant-design-vue/es/table'
import type { DetailColumnItem, IBaseDetailInfo, IColumns, IMap, PartitionColumnItem } from '@/types/common.type'
import { getTableDetail } from '@/services/table.service'
import { dateFormat } from '@/utils'

const emit = defineEmits<{
(e: 'setBaseDetailInfo', data: IBaseDetailInfo): void
(e: 'tableNotFound', info: { catalog: string; db: string; table: string }): void
}>()
const { t } = useI18n()
const route = useRoute()
const router = useRouter()

const STORAGE_TABLE_KEY = 'easylake-menu-catalog-db-table'

const params = computed(() => {
return {
Expand Down Expand Up @@ -79,14 +83,15 @@ const state = reactive({
})

async function getTableDetails() {
const requestParams = { ...params.value }
const { catalog, db, table } = requestParams
if (!catalog || !db || !table) {
return
}
try {
const { catalog, db, table } = params.value
if (!catalog || !db || !table) {
return
}
state.detailLoading = true
const result = await getTableDetail({
...params.value,
...requestParams,
})
const { pkList = [], tableType, partitionColumnList = [], properties, changeMetrics, schema, createTime, tableIdentifier, baseMetrics, tableSummary, comment } = result
state.baseDetailInfo = {
Expand Down Expand Up @@ -124,6 +129,20 @@ async function getTableDetails() {
setBaseDetailInfo()
}
catch (error) {
const errorMessage = (error as Error)?.message || ''
const isNotFoundError = /not exist|not found/i.test(errorMessage)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestions from AI:

There is a potential race condition in getTableDetails when the route changes while a previous request is still in flight. params is a computed value based on route.query, so during the await getTableDetail(...) call the user can switch from table A to table B, causing params.value to point to table B.

When the slow request for table A finally fails with "Table does not exist" / "table not found", the catch block reads catalogName, dbName, and tableName again from params.value. At that time params.value may already refer to table B, so we will:

  • remove the cached table key from localStorage,
  • emit tableNotFound for table B, and
  • redirect the user back to /tables.
  • This means a valid table B can be mistakenly treated as non-existent if a previous request for table A fails later, which is confusing for users.

Suggestion: take a snapshot of the parameters at the beginning of getTableDetails and use that snapshot consistently in both the request and the error handling, for example:

const getTableDetails = async () => {
  isLoading.value = true;
  const requestParams = { ...params.value };
  const { catalogName, dbName, tableName } = requestParams;
  try {
    const res = await getTableDetail({
      type: requestParams.tableType,
      name: tableName,
      database: dbName,
      catalog: catalogName,
    });
    tableDetails.value = res?.data?.details;
    getTableOptimizeInfo();
  } catch (error: any) {
    const e = error as AxiosError<any> & { data: { message?: string } };
    const message = e?.data?.message;
    if (message && (message.includes("Table does not exist") || message.includes("table not found"))) {
      localStorage.removeItem(STORAGE_TABLE_KEY);
      emit("tableNotFound", `${catalogName}/${dbName}/${tableName}`);
      router.replace({ path: "/tables", query: {} });
      return;
    }
  } finally {
    isLoading.value = false;
  }
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Fixed in f716e97 — params are now snapshot at the start of getTableDetails() so that both the API call and the catch block use the same values, preventing the race condition when the route changes during an in-flight request.


if (isNotFoundError) {
localStorage.removeItem(STORAGE_TABLE_KEY)

emit('tableNotFound', {
catalog: catalog as string,
db: db as string,
table: table as string,
})

router.replace({ path: '/tables', query: {} })
}
}
finally {
state.detailLoading = false
Expand Down
21 changes: 20 additions & 1 deletion amoro-web/src/views/tables/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,24 @@ export default defineComponent({
state.baseInfo = { ...baseInfo }
}

const handleTableNotFound = () => {
state.baseInfo = {
optimizingStatus: '',
records: '',
tableType: '',
tableName: '',
createTime: '',
tableFormat: '',
hasPartition: false,
healthScore: -1,
smallFileScore: 0,
equalityDeleteScore: 0,
positionalDeleteScore: 0,
comment: '',
} as IBaseDetailInfo
state.detailLoaded = false
}

const onChangeTab = (key: string | number) => {
const query = { ...route.query }
query.tab = key.toString()
Expand Down Expand Up @@ -210,6 +228,7 @@ export default defineComponent({
isIceberg,
hasSelectedTable,
setBaseDetailInfo,
handleTableNotFound,
goBack,
onChangeTab,
sidebarWidth,
Expand Down Expand Up @@ -262,7 +281,7 @@ export default defineComponent({
<div class="tables-main-body">
<a-tabs v-model:activeKey="activeKey" destroy-inactive-tab-pane @change="onChangeTab">
<a-tab-pane key="Details" :tab="$t('details')" force-render>
<UDetails ref="detailRef" @set-base-detail-info="setBaseDetailInfo" />
<UDetails ref="detailRef" @set-base-detail-info="setBaseDetailInfo" @table-not-found="handleTableNotFound" />
</a-tab-pane>
<a-tab-pane v-if="detailLoaded" key="Files" :tab="$t('files')">
<UFiles :has-partition="baseInfo.hasPartition" />
Expand Down