Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added java.lang.Byte and java.lang.Short Valuefier support #9134

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added java.lang.Byte and java.lang.Short Valuefier support #9134

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions logstash-core/src/main/java/org/logstash/Valuefier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ private static Map<Class<?>, Valuefier.Converter> initConverters() {
converters.put(RubyBignum.class, IDENTITY);
converters.put(RubyBigDecimal.class, IDENTITY);
converters.put(String.class, input -> RubyUtil.RUBY.newString((String) input));
converters.put(Byte.class, input -> RubyUtil.RUBY.newString(input.toString()));
Copy link
Member

Choose a reason for hiding this comment

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

@Twinkiee why don't we cast to int here and use LONG_CONVERTER as well? Seems more logical to me since a byte is a numeric type and not necessarily equivalent to a char in a String.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @original-brownbear. At the local Event level a byte is a number.

However, I looked at the IBM MQ Java docs. I wanted to know if the byte is considered a ASCII character or a numeric value.

This header MQIIH can return a char for TranState.
I saw others that returned byte[].
Via the CodedCharSetId, messages can be in UTF8 or CCS-2 (and variants).

@Twinkiee
Given that, at the event level, we don't know what the byte represents, it seems like the jms plugin is a better place to code for this.

Copy link
Author

@Twinkiee Twinkiee Feb 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@original-brownbear @guyboertje
It seems reasonable to me as well to convert the Byte as int as a general rule. That's how the java.lang.Byte#toString() threats the internal value anyway so there's really no point to behave differently.
I could/should've coded it in the first place, my bad.

On the other hand I'll try to investigate if it's actually possible for the IBM MQ to return a byte which is actually a char.
As far as I know, that's not the case. Sorry if it caused any confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Twinkiee

Any progress on your investigation of IBM MQ char as byte investigation?

Copy link
Author

Choose a reason for hiding this comment

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

@guyboertje
Hey, sorry for the late reply.
No, by my tests and investigations I couldn't find a byte converted into a char.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

@Twinkiee
Please make the update that @original-brownbear suggests then we can move this forward.

converters.put(Short.class, LONG_CONVERTER);
converters.put(Float.class, FLOAT_CONVERTER);
converters.put(Double.class, FLOAT_CONVERTER);
converters.put(
Expand Down