Replace CITUS_TABLE_WITH_NO_DIST_KEY checks with HasDistributionKey()

[onurctirtir]

Now that we will soon add another table type having DISTRIBUTE_BY_NONE
as distribution method and that we want the code to interpret such
tables mostly as distributed tables, let's make the definition of those
other two table types more strict by removing CITUS_TABLE_WITH_NO_DIST_KEY
macro.

And instead, use HasDistributionKey() check in the places where the
logic applies to all table types that have / don't have a distribution
key. In future PRs, we might want to convert some of those
HasDistributionKey() checks if logic only applies to Citus local /
reference tables, not the others.

And adding HasDistributionKey() also allows us to consider having
DISTRIBUTE_BY_NONE as the distribution method as a "table attribute"
that can apply to distributed tables too, rather something that
determines the table type.

[codecov]

Codecov Report

Merging #6743 (437f50b) into main (e3cf7ac) will increase coverage by 0.00%.
The diff coverage is 97.01%.

@@           Coverage Diff           @@
##             main    #6743   +/-   ##
=======================================
  Coverage   93.16%   93.17%           
=======================================
  Files         259      259           
  Lines       55912    55927   +15     
=======================================
+ Hits        52092    52108   +16     
+ Misses       3820     3819    -1     

[marcocitus]

would this be different for single shard tables?

[onurctirtir]

Probably no, or depends on what we want to do for single shard tables. But now that we don't have that table type at hand yet, this should be fine for the time being?

[marcocitus]

but wouldn't we just make things harder on ourselves?

Logic here says that it's ok to create a unqiue constraint if there's no distribution key. I think that's perfectly reasonable and will still be reasonable for single shard tables without a distribution key.

[marcocitus]

I don't quite understand how this change helps us with single shard tables.

Wouldn't a lot of the cases where we use CITUS_TABLE_WITH_NO_DIST_KEY apply to single shard tables in the same way?

[onurctirtir]

Now that we want to interpret null dist key tables as distributed, IsCitusTableType(DISTRIBUTED_TABLE) should return true for such tables too.

However, it would be confusing if IsCitusTableType(CITUS_TABLE_WITH_NO_DIST_KEY) also returns true for the same table. Because in that case, even the order of if conditions would be prone to bugs:

if (IsCitusTableType(DISTRIBUTED_TABLE))
{
  // then it's distributed table
}
else if (IsCitusTableType(CITUS_TABLE_WITH_NO_DIST_KEY))
{
..
}

// vs

if (IsCitusTableType(CITUS_TABLE_WITH_NO_DIST_KEY))
{
  // then it's not a distributed table
}
else if (IsCitusTableType(DISTRIBUTED_TABLE))
{
..
}

For this reason, I replaced CITUS_TABLE_WITH_NO_DIST_KEY with CITUS_LOCAL_OR_REFERENCE_TABLE and scoped its definition down to citus local tables / reference tables. For the codepaths where it makes more sense to act similar to citus local tables / reference tables, I'm planning to add necessary or conditions in future PRs.

[marcocitus]

That depends on whether you interpret CITUS_TABLE_WITH_NO_DIST_KEY as a table type or a table property.

I mostly interpret it as a table property, and the logic here makes sense to me for tables with that property.

On the other hand, the function IsCitusTableType. Perhaps we should switch to HasDistributionColumn?