Avoid UnknownAttributeReference error by eliminating Array#join

[marksiemers]

No description provided.

[marksiemers]

I've continued testing this, and it does not yet fix all cases.

This works when processing one record at a time, but when attempting in_batches the same error shows up in a different part of the code (calling pluck)

I believe it is this code:

    def pluck_columns(relation)
      if @pluck_columns.size == 1 # only the primary key
        column_values = relation.pluck(*@pluck_columns)
        return [column_values, column_values]
      end

      column_values = relation.pluck(*@pluck_columns)
      primary_key_index = @primary_key_index || -1
      primary_key_values = column_values.map { |values| values[primary_key_index] }

      serialize_column_values!(column_values)
      [column_values, primary_key_values]
    end

And using Arel.sql did not fix it, so something more in depth may be required.

[etiennebarrie]

Can you share a bit more about the relation you're passing to job-iteration (in collection)?

[marksiemers]

Sure thing, what is working using the code in this PR, but was not without this change is the following:

UPDATE:

For clarification, OrdinanceLookup and OrdinanceLookupBatch are ActiveRecord models, and ordinance_lookups is a has_many association on the OrdinanceLookupBatch model.

ORIGINAL

  def collection
    return OrdinanceLookup.none unless status.in?(DataSetJobStatus::VALID_STATUSES)

    batch = OrdinanceLookupBatch.find_by!(external_identifier: batch_external_identifier)
    batch.ordinance_lookups
  end

What still isn't working is:

  def collection
    return OrdinanceLookup.none unless status.in?(DataSetJobStatus::VALID_STATUSES)

    batch = OrdinanceLookupBatch.find_by!(external_identifier: batch_external_identifier)
    batch.ordinance_lookups.in_batches
  end

Perhaps it is that the collection is an association?

Do you think it would work to have:

batch = OrdinanceLookupBatch.find_by!(external_identifier: batch_external_identifier)
OrdinanceLookup.where(batch: batch)

[bray]

We're still seeing this. Any update on a fix?

[Mangara]

It's not entirely clear to me what this fixes. The code in #226 (comment) seems to be for a maintenance task, not a direct job-iteration job. Could you add a test that fails without this change and passes with it?

[marksiemers]

I have signed the CLA