Skip to content

StreamEntry support Binary#3803

Closed
thachlp wants to merge 2 commits intoredis:masterfrom
thachlp:stream-entry-support-binary
Closed

StreamEntry support Binary#3803
thachlp wants to merge 2 commits intoredis:masterfrom
thachlp:stream-entry-support-binary

Conversation

@thachlp
Copy link
Copy Markdown
Contributor

@thachlp thachlp commented Apr 6, 2024

Issue: #3566

Copy link
Copy Markdown
Contributor

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must not break the existing implementations.

@thachlp
Copy link
Copy Markdown
Contributor Author

thachlp commented Apr 6, 2024

We must not break the existing implementations.

Hi, @sazzad16, thanks for the review 🙇
How do we resolve this issue?

@sazzad16
Copy link
Copy Markdown
Contributor

sazzad16 commented Apr 6, 2024

How do we resolve this issue?

By creating new methods.


I just realized what your title says:

StreamEntry support Binary instead of String

Actually we want StreamEntry binary support while keeping the String ones.

@thachlp
Copy link
Copy Markdown
Contributor Author

thachlp commented Apr 6, 2024

How do we resolve this issue?

By creating new methods.

I just realized what your title says:

StreamEntry support Binary instead of String

Actually we want StreamEntry binary support while keeping the String ones.

Thanks, I will update.

@sazzad16 sazzad16 changed the title StreamEntry support Binary instead of String StreamEntry support Binary Apr 9, 2024
Copy link
Copy Markdown
Contributor Author

@thachlp thachlp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sazzad16, as I understand, there is xadd support Map<byte[], byte[]> hash already.

  public final CommandObject<byte[]> xadd(byte[] key, XAddParams params, Map<byte[], byte[]> hash) {
    return new CommandObject<>(addFlatMapArgs(commandArguments(XADD).key(key).addParams(params), hash),
        BuilderFactory.BINARY);
  }

@sazzad16
Copy link
Copy Markdown
Contributor

@thachlp StreamEntry has nothing to do with XADD.

@thachlp thachlp marked this pull request as draft April 19, 2024 14:16
@thachlp thachlp force-pushed the stream-entry-support-binary branch 2 times, most recently from 1ffa1d7 to 431f721 Compare April 19, 2024 14:44
@thachlp thachlp closed this Jun 27, 2024
@jfloodnet
Copy link
Copy Markdown

jfloodnet commented Jul 21, 2024

Now that this package supports binary value in stream entries, would someone be able to update spark redis to support binary columns also?

It is currently only Map<string, string> https://github.com/RedisLabs/spark-redis/blob/master/src/main/scala/com/redislabs/provider/redis/streaming/RedisStreamReceiver.scala#L108

Which causes the following error:

java.lang.ClassCastException: class org.apache.spark.unsafe.types.UTF8String cannot be cast to class [B (org.apache.spark.unsafe.types.UTF8String is in unnamed module of loader 'app'; [B is in module java.base of loader 'bootstrap') at org.apache.spark.sql.catalyst.expressions.BaseGenericInternalRow.getBinary(rows.scala:46)

If your stream entry schema has binary data:

    public static StructType redisSchema() {
        return new StructType(new StructField[]{
                DataTypes.createStructField("msg", DataTypes.BinaryType, true)
        });
    }

@sazzad16
Copy link
Copy Markdown
Contributor

@jfloodnet This PR is closed by OP without being merged.

@jfloodnet
Copy link
Copy Markdown

@jfloodnet This PR is closed by OP without being merged.

oh that's true. @thachlp what was the reason you closed this PR?

@thachlp
Copy link
Copy Markdown
Contributor Author

thachlp commented Jul 26, 2024

@jfloodnet I am just ambiguous about doing it, but If it is important, I will continue on it now, I will ask @sazzad16 for help.

@thachlp thachlp reopened this Jul 26, 2024
@thachlp thachlp force-pushed the stream-entry-support-binary branch from 480180b to 8582f4c Compare September 30, 2024 15:01
@thachlp thachlp force-pushed the stream-entry-support-binary branch from 8582f4c to d92e9d7 Compare September 30, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants