Decouples PhysicalTable name from Druid table name

[archolewa]

--Previously, the name of a PhysicalTable was one and the same as the
name of the associated datasource in Druid. However, this inhibits the
ability of Fili users to make creative use of Physical Tables to model
unusual Druid configurations.

--Therefore, the Fili name of a PhysicalTable has been decoupled from
the Druid name. The Fili name (name) is used to uniquely identify the
table in Fili, but the Druid name is the name serialized when building a
druid query. It is also the name used when querying Druid for the segment
metadata of a table.

[michael-mclawhorn]

Could we pick a not 'druid' name for the back end name so as not to create more rework when we get around to a second backend storage.

Maybe factTableName?

[cdeszaq]

Other than the trivial space issue on the toString change, this looks good. 👍

Personally, I'd rather leave this as referring to "druid", for consistency, and make 1 big change to remove Druid refs once we move those concerns into their own module. Until then, consistency seems better to me. That said, I don't feel strongly either way.

[cdeszaq]

Missing a space inside "druidName: "

[archolewa]

I thought about using a druid independent name as well, but I decided not to because right now, the PhysicalTable is a model of Druid tables, is coupled pretty tightly to Druid, and is going to end up getting reworked significantly anyway as a part of supporting multiple backends.

[michael-mclawhorn]

I'd like to dispute the claim that the PhysicalTable models a druid table.
I know that the comment line 31 says that, but not one line of code does.

Also, I'm of the opinion that getting off the druidisms will be easier to do if we start weaning and create the pressure for the change. Holding everything back until a major change leads to a) A massive set of PRs that nobody likes to review and b) continued investment in bad thinking about the system.

Changing PhysicalTable to not reflect a druid table but to reflect a view on fact data is SO VERY IMMINENT that it hurts.

[cdeszaq]

I've never found internal inconsistencies to exert enough pressure to actually cause change. Instead, I've seen such things generally cause larger snarls of inconsistencies due to making things harder to understand.

As I said before, I'm not particularly opinionated on this, but if we're going to change from Druid to FactSource, when that change happens, we should put in the due diligence effort to make as much of the shift in 1 go as we can reasonably achieve without undue searching and spelunking.

As to the imminence of the shift, I'm not aware of anyone actually planning to undertake that work in the next quarter, so I'm dubious of said imminence, but I would love to be proven wrong as to contributors' plans, since I think that would be a big feature boost.

[archolewa]

@michael-mclawhorn I'll change it. Just keep in mind that just because there's no code in PhysicalTable that explicitly ties it to a Druid table does not mean that it is not a model of a Druid table. That's only true if there is no code anywhere in our system that makes any assumptions, implicit or explicit that a physical table models a druid table. We won't know that until we actually go support multiple backends as first class citizens (though union data sources may also be enough to flesh out any such implicit assumptions). So until we know for sure, I usually err on the side of "We're claiming this models X, so by golly it models X."

[archolewa]

@michael-mclawhorn The field has been renamed.

[michael-mclawhorn]

👍 go for it