ARROW-6870: [C#] Add Support for Dictionary Arrays and Dictionary Encoding by HashidaTKS · Pull Request #10527 · apache/arrow

HashidaTKS · 2021-06-14T13:52:24Z

This is a implementation of DictionaryBatch (de)serialization for the streaming format.

The following features are missing for now, I plan to implement these features in another future PR.

The (de)serialization for the file format and Flight are not implemented yet
isDelta is not supported yet

github-actions · 2021-06-14T13:52:45Z

https://issues.apache.org/jira/browse/ARROW-6870

- Implement Dictionary serialization for ArrowStreamReader/Writer

HashidaTKS · 2021-06-14T14:39:09Z

cc: @eerhardt
Would you please review this when you have time?

eerhardt

This looks like a good start. Thanks @HashidaTKS!

Here's my first round of comments.

eerhardt · 2021-06-16T15:05:57Z

+
+        public ArrowReaderImplementation()
+        {
+            _dictionaryMemo = new DictionaryMemo();


Can we lazy load this until it is needed? It currently allocates 3 Dictionary<K, V> objects, which are unnecessary if the payload doesn't contain any dictionaries.

I have created a LazyCreator class and changed DictionaryMemo _dictionaryMemo to LazyCreator<DictionaryMemo> _lazyDictionaryMemo.
However, I am wondering whether this is the best way to lazy load because it need allocate a LazyCreator object.
Is there any other good idea?

I wouldn't use create a new class for this, .NET has https://docs.microsoft.com/dotnet/api/system.lazy-1 already for it.

Even better is to not use Lazy<T> at all, and just check for null before using it. You can make a private property to make this really easy:

private DictionaryMemo _dictionaryMemo; private DictionaryMemo DictionaryMemo => _dictionaryMemo ??= new DictionaryMemo(); // later in code that needs to use DictonaryMemo: DictionaryMemo.GetDictionaryType(id);

This way, nothing new is allocated when we don't see a Dictionary batch in the payload.

Thank you, I reimplemented them with the way 2.

eerhardt · 2021-06-16T18:32:15Z


        private readonly IMemoryOwner<byte> _memoryOwner;
-        private readonly IList<IArrowArray> _arrays;
+        internal readonly IReadOnlyList<IArrowArray> _arrays;


Instead of exposing this field, can you make an internal property?

Suggested change

internal readonly IReadOnlyList<IArrowArray> _arrays;

private readonly IList<IArrowArray> _arrays;

internal IReadOnlyList<IArrowArray> Arrays => _arrays;

Fixed it.

There was already a public property named Arrays so I named the internal property _Arrays.

eerhardt · 2021-06-16T19:02:55Z


            _fieldTypeBuilder = new ArrowTypeFlatbufferBuilder(Builder);
            _options = options ?? IpcOptions.Default;
+            _dictionaryMemo = new DictionaryMemo();


Can this be lazy-initialized until it is needed? That way every ArrowStreamWriter doesn't need to create it, if they don't use Dictionaries.

eerhardt · 2021-06-16T19:19:57Z

+            _idToDictionary[id] = dictionary;
+        }
+
+        public void AddDictionaryDelta(long id, IArrowArray dictionary)


Can this be deleted until it is supported? It is dead code.

This hasn't been deleted (yet)

Sorry, I didn't commit the change.

- Separate ArrayData constructors - Remove unused methods / constructors - Load DictionaryMemo lazily - Hide _arrays of RecordBatch - Fix tests

HashidaTKS · 2021-06-17T17:43:30Z

                reader.ReadNextRecordBatch();

-                Assert.Equal(1, memoryPool.Statistics.Allocations);
+                Assert.Equal(2, memoryPool.Statistics.Allocations);


Memo:
_allocator is called when reading dictionary batches and record batches.
The expected value of memoryPool.Statistics.Allocations is 2 at this context.

Why change this test at all? If you want to add a new test for dictionaries - we should add a new one, and leave the existing test.

I fixed it to use [Theory].

HashidaTKS · 2021-06-17T18:03:04Z

@eerhardt
Thank you for the review!

I have reflected the feedback.

- Change a LazyCreator constructor position

- Fix a typo

- Slight refactoring for LazyCreator.Instance

HashidaTKS · 2021-06-19T04:18:01Z

Added some minor changes.

- refactor ArrayData constructors

HashidaTKS · 2021-06-19T06:08:45Z

Modified ArrayData constructors.

I'm sorry to commit many times after requesting a review.

eerhardt · 2021-06-30T13:50:02Z

Sorry for the delay @HashidaTKS. I didn't forget about this. I will get back to it this week, hopefully sometime today.

HashidaTKS · 2021-07-01T08:18:13Z

@eerhardt
No problem, I'm in no hurry. Please take your time.

eerhardt · 2021-07-01T16:56:44Z

                reader.ReadNextRecordBatch();

-                Assert.Equal(1, memoryPool.Statistics.Allocations);
+                Assert.Equal(2, memoryPool.Statistics.Allocations);


Why change this test at all? If you want to add a new test for dictionaries - we should add a new one, and leave the existing test.

eerhardt · 2021-07-01T16:59:41Z

+        internal IReadOnlyList<IArrowArray> _Arrays => (IReadOnlyList<IArrowArray>)_arrays;
+
        private readonly IMemoryOwner<byte> _memoryOwner;
        private readonly IList<IArrowArray> _arrays;


Suggested change

internal IReadOnlyList<IArrowArray> _Arrays => (IReadOnlyList<IArrowArray>)_arrays;

private readonly IMemoryOwner<byte> _memoryOwner;

private readonly IList<IArrowArray> _arrays;

internal IReadOnlyList<IArrowArray> ArrayList => _arrays;

private readonly IMemoryOwner<byte> _memoryOwner;

private readonly List<IArrowArray> _arrays;

Using the coding style from https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/coding-style.md, only fields should have _ prefix.

We shouldn't need to cast here. Just have the private field as a List<IArrowArray>.

I fixed it.

eerhardt · 2021-07-01T17:03:50Z

        public Stream BaseStream { get; }
        private readonly bool _leaveOpen;
        private readonly MemoryAllocator _allocator;
+        private protected bool HasReadInitialDictionary { get; set; }


Suggested change

private protected bool HasReadInitialDictionary { get; set; }

private bool HasReadInitialDictionary { get; set; }

Nothing outside this class uses this property.

I fixed it.

eerhardt · 2021-07-01T17:04:19Z

+
+        public ArrowStreamReaderImplementation(Stream stream, MemoryAllocator allocator, bool leaveOpen) : base()


Suggested change

public ArrowStreamReaderImplementation(Stream stream, MemoryAllocator allocator, bool leaveOpen) : base()

public ArrowStreamReaderImplementation(Stream stream, MemoryAllocator allocator, bool leaveOpen)

Needless extra line.

No need for the explicit call to base.

I fixed it.

eerhardt · 2021-07-01T17:05:34Z

            }
        }

+        protected void ReadInitialDictionaries()


Suggested change

protected void ReadInitialDictionaries()

private void ReadInitialDictionaries()

I fixed it.

eerhardt · 2021-07-01T19:29:13Z

+                childFields[i] = FieldFromFlatbuffer(childFlatbufField.Value, lazyDictionaryMemo);
+            }
+
+            Flatbuf.DictionaryEncoding? de = flatbufField.Dictionary;


(nit) Can you spell out de? It doesn't make a great reading experience to figure out what de means.

Suggested change

Flatbuf.DictionaryEncoding? de = flatbufField.Dictionary;

Flatbuf.DictionaryEncoding? dictionaryEncoding = flatbufField.Dictionary;

I fixed it.

eerhardt · 2021-07-01T19:30:11Z

+                Flatbuf.Int? indexTypeAsInt = de.Value.IndexType;
+                if (!indexTypeAsInt.HasValue)
+                {
+                    throw new InvalidDataException("Dictionary type not defined");


Would this be a more correct message?

Suggested change

throw new InvalidDataException("Dictionary type not defined");

throw new InvalidDataException("Dictionary IndexType not defined");

I fixed it.

eerhardt · 2021-07-01T19:31:56Z

        }

-        internal static Schema GetSchema(Flatbuf.Schema schema)
+        internal static Schema GetSchema(Flatbuf.Schema schema, LazyCreator<DictionaryMemo> lazyDictionaryMemo)


Along with the comment about removing LazyCreator, this could be changed to:

Suggested change

internal static Schema GetSchema(Flatbuf.Schema schema, LazyCreator<DictionaryMemo> lazyDictionaryMemo)

internal static Schema GetSchema(Flatbuf.Schema schema, ref DictionaryMemo dictionaryMemo)

And then if this code needs to use the dictionaryMemo and it is null, it gets created when it is needed. And the ref gets set.

I fixed it.

eerhardt · 2021-07-01T19:37:15Z

    {
-        public static RecordBatch CreateSampleRecordBatch(int length)
+        //TODO: Remove the createDictionaryArray argument after all writer/reader supports DictionaryType serialization
+        public static RecordBatch CreateSampleRecordBatch(int length, bool createDictionaryArray = false)


A general comment about createDictionaryArray - I think the existing tests should be left alone. That way we are still testing as much as we can without any dictionaries. Then we create new tests for dictionary support.

An easy way to do this is to change a bunch of tests from [Fact] to [Theory] and pass in true and false into the test for bool createDictoinaryArray. That way the test runs twice, once with a dictionary, and once without. That should give us the best test coverage.

I understand.
I fixed tests to use [Theory]

- Fix access modifiers - Fix tests - Change the way of the lazy creation for dictionaryMemo

HashidaTKS

@eerhardt
Thank you for the review!
I have responded to what you have pointed out.

Fix access modifiers and the code format
- Sorry for my carelessness...
Fix tests
Change the way of the lazy creation for dictionaryMemo

HashidaTKS · 2021-07-03T18:26:46Z

    public static class TestData
    {
-        public static RecordBatch CreateSampleRecordBatch(int length)
+        //TODO: Remove the createDictionaryArray argument after all writer/reader supports DictionaryType serialization


I was planning to always create a dictionary array after ArrowFileWriter/Reader support DictionaryType serialization.
But considering your feedback comments, now I think it is better to leave createDictionaryArray even after that.
I will remove this TODO.

HashidaTKS · 2021-07-04T14:16:34Z

        public Stream BaseStream { get; }
        private readonly bool _leaveOpen;
        private readonly MemoryAllocator _allocator;
+        private protected bool HasReadInitialDictionary { get; set; }


I fixed it.

HashidaTKS · 2021-07-04T19:35:13Z

                reader.ReadNextRecordBatch();

-                Assert.Equal(1, memoryPool.Statistics.Allocations);
+                Assert.Equal(2, memoryPool.Statistics.Allocations);


I fixed it to use [Theory].

HashidaTKS · 2021-07-04T19:35:20Z

+        internal IReadOnlyList<IArrowArray> _Arrays => (IReadOnlyList<IArrowArray>)_arrays;
+
        private readonly IMemoryOwner<byte> _memoryOwner;
        private readonly IList<IArrowArray> _arrays;


I fixed it.

HashidaTKS · 2021-07-04T19:37:04Z

            }
        }

+        protected void ReadInitialDictionaries()


I fixed it.

HashidaTKS · 2021-07-04T19:37:08Z

+
+        public ArrowStreamReaderImplementation(Stream stream, MemoryAllocator allocator, bool leaveOpen) : base()


I fixed it.

HashidaTKS · 2021-07-04T19:40:49Z

    {
-        public static RecordBatch CreateSampleRecordBatch(int length)
+        //TODO: Remove the createDictionaryArray argument after all writer/reader supports DictionaryType serialization
+        public static RecordBatch CreateSampleRecordBatch(int length, bool createDictionaryArray = false)


I understand.
I fixed tests to use [Theory]

HashidaTKS · 2021-07-04T19:42:09Z

    public static class TestData
    {
-        public static RecordBatch CreateSampleRecordBatch(int length)
+        //TODO: Remove the createDictionaryArray argument after all writer/reader supports DictionaryType serialization


I removed the TODO comment.

HashidaTKS · 2021-07-04T19:43:00Z

        public void Ctor_LeaveOpenDefault_StreamClosedOnDispose()
        {
-            RecordBatch originalBatch = TestData.CreateSampleRecordBatch(length: 100);
+            RecordBatch originalBatch = TestData.CreateSampleRecordBatch(length: 100, createDictionaryArray: true);


I fixed it.

- change the place of the HasCreatedDictionaryMemo property

- Remove needless extra lines

eerhardt · 2021-07-08T19:54:54Z

+            HasReadInitialDictionary = true;
+        }
+
+        private async ValueTask ReadInitialDictionariesAsync(CancellationToken cancellationToken)


(nit) the other methods in this file have the Async method first, and then the synchronous method after it. Can this go above ReadInitialDictionaries()?

eerhardt · 2021-07-08T20:13:21Z

+
+            ReadInitialDictionaries();
+
+            return ReadArrowObject();


The spec says

DictionaryBatch and RecordBatch messages may be interleaved

This appears to only support DictionaryBatches above RecordBatches. If we read all the "initial" dictionaries, and then read a RecordBatch, the next time ReadRecordBatch() is called, if there is a DictionaryBatch, null will be returned. Is that intentional?

Sorry, it slipped my mind. That is not intentional.
I modified to be able to read interleaved dictionaries.

During the modification, I realized that ReadInitialDictionariesAsync and ReadInitialDictionaries are needless, so I removed them.

Related to this, ArrowStreamWriter does not support writing interleaved dictionaries for now.
I would like to implement it in another PR because this PR will become too large.
How is that?

I tested reading interleaved dictionaries by creating a test file with python and reading the test file with C#, and it seemed to work fine.
The code for creating the test file is below, this code is based on the test code of the python implementation.

https://github.com/apache/arrow/blob/master/python/pyarrow/tests/test_ipc.py#L417

import pyarrow as pa ty = pa.dictionary(pa.int8(), pa.utf8()) data = [["foo", "foo", None], ["foo", "bar", "foo"], ["foo", "bar"], ["foo", None, "bar", "quux"], ["bar", "quux"], ] batches = [ pa.RecordBatch.from_arrays([pa.array(v, type=ty)], names=['dicts']) for v in data] schema = batches[0].schema def write_batches(): with pa.RecordBatchStreamWriter("./dictionary_batch_test.batch", schema = schema) as writer: for batch in batches: writer.write_batch(batch) st = write_batches()

We should add C# tests for this after ArrowStreamWriter supports writing interleaved dictionaries.

I would like to implement it in another PR because this PR will become too large.
How is that?

That is perfectly reasonable to me. I think the current functionality is sufficient for the first PR.

I tested reading interleaved dictionaries by creating a test file with python

In my spare time, I've been trying to set up the integration tests, so we can test the C# implementation against the other languages. Hopefully I can get the initial PR for that up soon.

- Avoid a needless allocation - Remove a needless ctor

- Support reading the replacement dictionaries - Add tests for writing and reading dictionaries used in NestedType arrays - Fix a bug when reading dictionaries used in NestedType arrays

- Regard indexType as signed int32 if it is null

- DictionaryArray with children - Add a test for this - Fix a serialization bug about this

HashidaTKS

@eerhardt
Thank you!

I have responded to what you have pointed out.

Avoid needless allocation
Remove a needless ctor
Support reading the replacement dictionaries
Add tests for writing and reading dictionaries used in NestedType arrays
Add tests for writing and reading ListType dictionaries
Fix a bug when reading dictionaries used in NestedType arrays
Fix a bug when writing ListType dictionaries
Regard indexType as signed int32 if it is null

HashidaTKS · 2021-07-11T17:43:59Z

+
+            ReadInitialDictionaries();
+
+            return ReadArrowObject();


Sorry, it slipped my mind. That is not intentional.
I modified to be able to read interleaved dictionaries.

During the modification, I realized that ReadInitialDictionariesAsync and ReadInitialDictionaries are needless, so I removed them.

Related to this, ArrowStreamWriter does not support writing interleaved dictionaries for now.
I would like to implement it in another PR because this PR will become too large.
How is that?

HashidaTKS · 2021-07-13T00:07:54Z

+
+            ReadInitialDictionaries();
+
+            return ReadArrowObject();


I tested reading interleaved dictionaries by creating a test file with python and reading the test file with C#, and it seemed to work fine.
The code for creating the test file is below, this code is based on the test code of the python implementation.

https://github.com/apache/arrow/blob/master/python/pyarrow/tests/test_ipc.py#L417

import pyarrow as pa ty = pa.dictionary(pa.int8(), pa.utf8()) data = [["foo", "foo", None], ["foo", "bar", "foo"], ["foo", "bar"], ["foo", None, "bar", "quux"], ["bar", "quux"], ] batches = [ pa.RecordBatch.from_arrays([pa.array(v, type=ty)], names=['dicts']) for v in data] schema = batches[0].schema def write_batches(): with pa.RecordBatchStreamWriter("./dictionary_batch_test.batch", schema = schema) as writer: for batch in batches: writer.write_batch(batch) st = write_batches()

We should add C# tests for this after ArrowStreamWriter supports writing interleaved dictionaries.

HashidaTKS · 2021-08-05T00:21:54Z

Cc: @eerhardt

Would you please re-review this when you have time?

eerhardt

I just had a couple very minor comments. Once those are addressed, I think we can merge this.

Thank you for the great work here, @HashidaTKS! And thank you for your patience (I had been busy).

- Fix minor issues

HashidaTKS · 2021-08-09T02:01:30Z

@eerhardt
I have addressed commented issues.

I appreciate your review and support!

- do-while -> while

github-actions Bot added the Component: C# label Jun 14, 2021

Support Dictionary

0763588

- Implement Dictionary serialization for ArrowStreamReader/Writer

HashidaTKS force-pushed the ARROW-6870 branch from 666e7ed to 0763588 Compare June 14, 2021 13:54

HashidaTKS mentioned this pull request Jun 14, 2021

Apache ArrowのDictionary Messagesを使用する。 groonga/groonga#1181

Open

eerhardt reviewed Jun 16, 2021

View reviewed changes

ARROW-6870: Support Dictionary

8544064

- Separate ArrayData constructors - Remove unused methods / constructors - Load DictionaryMemo lazily - Hide _arrays of RecordBatch - Fix tests

HashidaTKS force-pushed the ARROW-6870 branch from 4765f83 to 8544064 Compare June 17, 2021 17:12

HashidaTKS commented Jun 17, 2021

View reviewed changes

ARROW-6870: Support Dictionary

5e2c17d

- Change a LazyCreator constructor position

HashidaTKS requested a review from eerhardt June 17, 2021 18:37

HashidaTKS added 2 commits June 19, 2021 05:27

ARROW-6870: Support Dictionary

79b0015

- Fix a typo

ARROW-6870: Support Dictionary

4d5745b

- Slight refactoring for LazyCreator.Instance

HashidaTKS force-pushed the ARROW-6870 branch from 00bf214 to 4d5745b Compare June 19, 2021 04:16

ARROW-6870: Support Dictionary

4ffc321

- refactor ArrayData constructors

eerhardt reviewed Jul 1, 2021

View reviewed changes

ARROW-6870: Support Dictionary

0e19500

- Fix access modifiers - Fix tests - Change the way of the lazy creation for dictionaryMemo

HashidaTKS commented Jul 4, 2021

View reviewed changes

HashidaTKS added 2 commits July 5, 2021 10:25

ARROW-6870: Support Dictionary

fae6e28

- change the place of the HasCreatedDictionaryMemo property

ARROW-6870: Support Dictionary

57d844d

- Remove needless extra lines

HashidaTKS requested a review from eerhardt July 8, 2021 14:48

eerhardt reviewed Jul 8, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs Outdated

eerhardt reviewed Jul 8, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs Outdated

eerhardt reviewed Jul 8, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs Outdated

eerhardt reviewed Jul 8, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs Outdated

eerhardt reviewed Jul 8, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs

HashidaTKS added 4 commits July 11, 2021 09:32

ARROW-6870: Support Dictionary

08fc153

- Avoid a needless allocation - Remove a needless ctor

ARROW-6870: Support Dictionary

f7de403

- Support reading the replacement dictionaries - Add tests for writing and reading dictionaries used in NestedType arrays - Fix a bug when reading dictionaries used in NestedType arrays

ARROW-6870: Support Dictionary

30c0cd9

- Regard indexType as signed int32 if it is null

ARROW-6870: Support Dictionary

7597029

- DictionaryArray with children - Add a test for this - Fix a serialization bug about this

HashidaTKS commented Jul 13, 2021

View reviewed changes

HashidaTKS requested a review from eerhardt July 13, 2021 00:42

eerhardt reviewed Aug 7, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs Outdated

eerhardt reviewed Aug 7, 2021

View reviewed changes

Comment thread csharp/src/Apache.Arrow/Ipc/ArrowStreamReaderImplementation.cs Outdated

eerhardt approved these changes Aug 7, 2021

View reviewed changes

ARROW-6870: Support Dictionary

487fc01

- Fix minor issues

ARROW-6870: Support Dictionary

6bdd277

- do-while -> while

eerhardt approved these changes Aug 9, 2021

View reviewed changes

eerhardt closed this in 0f1c3a3 Aug 9, 2021

asfimport mentioned this pull request Aug 9, 2021

[C#] Add Support for Dictionary Arrays and Dictionary Encoding #23198

Closed

	private protected bool HasReadInitialDictionary { get; set; }
	private bool HasReadInitialDictionary { get; set; }


		public ArrowStreamReaderImplementation(Stream stream, MemoryAllocator allocator, bool leaveOpen) : base()

	protected void ReadInitialDictionaries()
	private void ReadInitialDictionaries()

	Flatbuf.DictionaryEncoding? de = flatbufField.Dictionary;
	Flatbuf.DictionaryEncoding? dictionaryEncoding = flatbufField.Dictionary;

	throw new InvalidDataException("Dictionary type not defined");
	throw new InvalidDataException("Dictionary IndexType not defined");

	internal static Schema GetSchema(Flatbuf.Schema schema, LazyCreator<DictionaryMemo> lazyDictionaryMemo)
	internal static Schema GetSchema(Flatbuf.Schema schema, ref DictionaryMemo dictionaryMemo)

Uh oh!

Conversation

HashidaTKS commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 14, 2021

Uh oh!

HashidaTKS commented Jun 14, 2021

Uh oh!

eerhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HashidaTKS Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HashidaTKS commented Jun 17, 2021

Uh oh!

HashidaTKS commented Jun 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HashidaTKS commented Jun 19, 2021

Uh oh!

eerhardt commented Jun 30, 2021

Uh oh!

HashidaTKS commented Jul 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

HashidaTKS commented Jun 14, 2021 •

edited

Loading

HashidaTKS Jun 17, 2021 •

edited

Loading

HashidaTKS commented Jun 19, 2021 •

edited

Loading

HashidaTKS left a comment •

edited

Loading