dolthub / dolthub/dolt

Implement `TableIdNode` wherever needed and correctly assign `TableId`s

Open
#11,249 0 comments 0 reactions 0 assignees View on GitHub
analyzer
Dominant language
Go
Stars
24.4k
Forks
873
Avg merge
1d 5h
Merged PRs (30d)
108

Description

[This comment for `TableId`](https://github.com/dolthub/go-mysql-server/blob/eee792a22ff6a32651f4a27946b0e5ee5a8a5c36/sql/column.go#L160) shows an intention to add `TableId`s to more nodes. Using `TableId`s instead of table name strings is definitely something we should be moving towards -- it allows us to easily disambiguate between tables with the same name from different databases and we can also use `TableId`s and `FastIntSet`s to avoid costly string matching.

However, there are lots of places where `TableId`s are not correctly assigned, if at all. In order to make better use of `TableId`s, they need to actually be assigned correctly. There are also nodes that implement `TableIdNode` when they probably shouldn't (#10443)

`TableId` and `TableIdNode` should also be refactored in a more organized way. It also makes more sense for `TableIdNode` to be part of the `sql` package instead of the `plan` package. It also doesn't make sense that `TableId` is defined in `sql/column.go`

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the linked TableId comment in sql/column.go, then inspect TableIdNode implementations in the sql and plan packages and review issue #10443. Map where TableIds are missing or incorrectly assigned and which nodes implement TableIdNode. Done means the definitions are organized in the intended package and all relevant nodes assign TableIds correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.