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.

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

[AMORO-2215] Optimize the speed of searching for tables in the tables navigation bar #2452

Merged
merged 13 commits into from
Mar 29, 2024
Merged
54 changes: 47 additions & 7 deletions ams/dashboard/src/components/tables-sub-menu/TablesMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
</template>

<script lang="ts">
import { defineComponent, onBeforeMount, reactive, toRefs } from 'vue'
import { defineComponent, onBeforeMount, reactive, toRefs, computed } from 'vue'
import {
// PlusOutlined,
SearchOutlined,
Expand Down Expand Up @@ -137,11 +137,31 @@ export default defineComponent({
loading: false,
tableLoading: false,
databaseList: [] as IMap<string>[],
tableList: [] as IMap<string>[]
tableList: [] as IMap<string>[],
allDatabaseListLoaded: [] as IMap<string>[],
allTableListLoaded: [] as IMap<string>[]
})
const storageTableKey = 'easylake-menu-catalog-db-table'
const storageCataDBTable = JSON.parse(localStorage.getItem(storageTableKey) || '{}')

const filteredDatabases = computed(() => {
if (!state.allDatabaseListLoaded) {
return []
}
return state.allDatabaseListLoaded.filter((ele) => {
return ele.label.includes(state.DBSearchInput)
})
})

const filteredTables = computed(() => {
if (!state.allTableListLoaded) {
return []
}
return state.allTableListLoaded.filter((ele) => {
return ele.label.includes(state.tableSearchInput)
})
})

const placeholder = reactive(usePlaceholder())

const handleSearch = (type: string) => {
Expand Down Expand Up @@ -171,6 +191,7 @@ export default defineComponent({
}
state.database = item.id
state.tableName = ''
state.allTableListLoaded.length = 0
getAllTableList()
}

Expand All @@ -185,6 +206,8 @@ export default defineComponent({
state.curCatalog = value
state.databaseList.length = 0
state.tableList.length = 0
state.allDatabaseListLoaded.length = 0
state.allTableListLoaded.length = 0
getAllDatabaseList()
}
const addDatabase = () => {
Expand Down Expand Up @@ -247,6 +270,11 @@ export default defineComponent({
if (!state.curCatalog) {
return
}
if (state.allDatabaseListLoaded.length) {
state.databaseList = filteredDatabases
return
}

state.loading = true
getDatabaseList({
catalog: state.curCatalog,
Expand All @@ -256,11 +284,14 @@ export default defineComponent({
id: ele,
label: ele
}))
if (state.databaseList.length && !isSearch) {
const index = state.databaseList.findIndex(ele => ele.id === storageCataDBTable.database)
// ISSUE 2413: If the current catalog is not the one in the query, the first db is selected by default.
state.database = index > -1 ? storageCataDBTable.database : state.curCatalog === (route.query?.catalog)?.toString() ? ((route.query?.db)?.toString() || state.databaseList[0].id || '') : state.databaseList[0].id || ''
getAllTableList()
if (!isSearch) {
state.allDatabaseListLoaded = [...state.databaseList]
if (state.databaseList.length) {
const index = state.databaseList.findIndex(ele => ele.id === storageCataDBTable.database)
// ISSUE 2413: If the current catalog is not the one in the query, the first db is selected by default.
state.database = index > -1 ? storageCataDBTable.database : state.curCatalog === (route.query?.catalog)?.toString() ? ((route.query?.db)?.toString() || state.databaseList[0].id || '') : state.databaseList[0].id || ''
getAllTableList()
}
}
}).finally(() => {
state.loading = false
Expand All @@ -271,6 +302,11 @@ export default defineComponent({
if (!state.curCatalog || !state.database) {
return
}
if (state.allTableListLoaded.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if the tableList here is still old after switching database, if yes, maybe you need to save a mapping from database to tableList?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, in the const handleClickDb = (item: IDatabaseItem) function, i have set state.allTableListLoaded.length = 0.

state.tableList = filteredTables
return
}

state.tableLoading = true
state.tableList.length = 0
getTableList({
Expand All @@ -283,10 +319,14 @@ export default defineComponent({
label: ele.name,
type: ele.type
}))
if (state.tableSearchInput === '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest moving the filtering of databases and tables from the backend to the frontend and the HTTP API always returns all databases and all tables. Then there will be no need for additional checks on whether allTableListLoaded should be set.

BTW, removing the filtering from the HTTP API won't incur additional overhead on the backend, because the filtering takes effect at the final step of returning results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, filtering of databases and tables is acted in the frontend now. We only need load all databases or tables for the first time. Afterwards, keyword filtering will not request HTTP API.

state.allTableListLoaded = [...state.tableList]
}
}).finally(() => {
state.tableLoading = false
})
}

onBeforeMount(() => {
const { database, tableName } = storageCataDBTable
state.database = database
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
* limitations under the License.
*/

import{v as n,x as r,r as _,o as p,e as d,z as a,u as t,h as o,a5 as i,y as l,a1 as u,p as m,a as f}from"./index-bS3EFDLG.js";/* empty css */const h=n({name:"Page404"}),x=e=>(m("data-v-5c997eea"),e=e(),f(),e),g={class:"g-flex-center"},v={class:"g-flex-col container"},k=x(()=>a("p",{class:"title"},"404 - Not Found",-1));function $(e,B,I,N,S,b){const s=u,c=_("router-link");return p(),d("div",g,[a("div",v,[k,t(c,{to:"/",style:{"text-align":"center"}},{default:o(()=>[t(s,{class:"button g-mt-32"},{default:o(()=>[i(l(e.$t("backHome")),1)]),_:1})]),_:1})])])}const V=r(h,[["render",$],["__scopeId","data-v-5c997eea"]]);export{V as default};
import{v as n,x as r,r as _,o as p,e as d,z as a,u as t,h as o,a5 as i,y as l,a1 as u,p as m,a as f}from"./index-ydzaPdn1.js";/* empty css */const h=n({name:"Page404"}),x=e=>(m("data-v-5c997eea"),e=e(),f(),e),g={class:"g-flex-center"},v={class:"g-flex-col container"},k=x(()=>a("p",{class:"title"},"404 - Not Found",-1));function $(e,B,I,N,S,b){const s=u,c=_("router-link");return p(),d("div",g,[a("div",v,[k,t(c,{to:"/",style:{"text-align":"center"}},{default:o(()=>[t(s,{class:"button g-mt-32"},{default:o(()=>[i(l(e.$t("backHome")),1)]),_:1})]),_:1})])])}const V=r(h,[["render",$],["__scopeId","data-v-5c997eea"]]);export{V as default};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading