ktmud edited a comment on pull request #11714: URL: https://github.com/apache/incubator-superset/pull/11714#issuecomment-738459267
@mistercrunch I think for things as heavy as that requiring a db migration, a little bit more instrumentation is warranted. I'd vote for revert before this goes out to more users. My two cents on the value of the added columns: 1. As John commented, `path_no_int` is not very useful because you can easily do the same RegExp matching and cleaning up in SQL (not the case for `path_no_param` that uses the actual route pattern, though). 2. The other two columns are actually under the same category as they could both have a 1:1 mapping to the existing column `action`. Since it's rare to run analytic queries on the production database because most likely you would analyze the logs in some kind of data warehouse, a little bit extra ETL work is probably more favorable in the spirit of keeping the production database lean and performant. I agree both `path` and `ref` (maybe call this `object_ref` to disambiguate with `referrer`?) are useful information. It's also valuable to record point-at-time information even if we could manually curate the mapping. So adding them to `json` sounds like a good idea. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
