Propose changes to record summaries for beta #114
Conversation
|
@kgodey @mathemancer @pavish I've assigned you so that you can review these plans. Let me know if you have any questions, concerns, or critique. |
There was a problem hiding this comment.
This all seems fine to me. I think the syntax changes are fine as long as the UX makes sense. I know the UX is still TBD, but when we do have it figured out, I want to make sure the UX does the following:
(1) provides the user a GUI way to select columns, including transitive columns.
(2) explains the syntax to the user in case they just want to enter text.
A nice to have would be helping the user understand how their syntax is wrong if there's some sort of syntactical error.
There was a problem hiding this comment.
I remember having some arguments collegial discussions at the beginning of the record summary work wherein my position was that the data needed to fill record summaries was already gettable though the joinable_tables and associated infrastructure in the data explorer. That's still true, and I still think we should reuse as much of that stuff as possible. I think, unless I'm missing some important details, that this should be well aligned with your goals of making these more general and query-like for extensibility in the long run. It also avoids having two big heavy machines dedicated to automatically joining tables together and using the results for some kind of display.
For this spec specifically, the only change (I think) needed is to use the current join path data structure for the join specifications, and to emphasize that the front end is responsible for putting names to IDs for showing the user (using info returned from the back end, of course). Then, if we're doing that, then SQL-style quoting and escaping isn't necessary.
| ```json | ||
| [ | ||
| [2], | ||
| ", by ", | ||
| [10, 2], | ||
| " ", | ||
| [10, 3] | ||
| ] | ||
| ``` | ||
|
|
||
| That means: first include a reference to the column in the base table having attnum 2; then include a string literal, then include a reference to a column that can be found by joining on the FK at column 10 and finding column with attnum 2 in the joined table... and so on. |
There was a problem hiding this comment.
I think we should reuse the fkey_path and/or jp_path structures we already have defined for holding this info. Out of curiosity , are you planning to be able to go multiple joins deep? In that case I really think we should use the more involved structures. They're more flexible in the long-run since they can handle arbitrary joins (at least in the case of jp_path). It also lets us reuse some machinery we've already build for getting the needed info (see 1_msar_joinable_tables.sql for example).
|
This pull request has not been updated in 45 days and is being marked as stale. It will automatically be closed in 30 days if not updated by then. |
This PR describes my plans for the changes to record summaries for beta.
Rendered